linux-ext4 - Re: [PATCH -v3] vfs: add releasepages hooks to block devices which can be used by file systems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 18 Dec 2008 14:15:25 +0900
From:	Toshiyuki Okajima <toshi.okajima@...fujitsu.com>
To:	Jan Kara <jack@...e.cz>
CC:	"Theodore Ts'o" <tytso@....edu>,
	Ext4 Developers List <linux-ext4@...r.kernel.org>,
	linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH -v3] vfs: add releasepages hooks to block devices which
 can be used by file systems

Hi,
 >   Hello,
 >
 > > > From: Toshiyuki Okajima <toshi.okajima@...fujitsu.com>
 > > >
 > > > Implement blkdev_releasepage() to release the buffer_heads and page
 > > > after we release private data which belongs to a client of the block
 > > > device, such as a filesystem.
 > > >
 > > > blkdev_releasepage() call the client's releasepage() which is
 > > > registered by blkdev_register_client_releasepage() to release its
 > > > private data.
 >   Yes, this is IMO the right fix. I'm just wondering about the fact that we
 > can't block in the client_releasepage(). That seems to be caused by the fact
 > that we need to be protected against client_releasepage() callback changes
 > which essentially means umount, right? I'm not saying I have a better solution
 > but introducing such limitation seems stupid just because of umount...
 >
 > 									Honza
 >

Difference between v2 and v3 in blkdev_releasepage:
<		ret = (*ei->client_releasepage)(ei->client, page, wait);
<	else
--
 >		/*
 >		 * Since we are holding a spinlock (ei->client_lock),
 >		 * make sure the client_releasepage function
 >		 * understands that it must not block.
 >		 */
 >		ret = (*ei->client_releasepage)(ei->client, page,
 >						wait & ~__GFP_WAIT);
 >	else

Ask for clarification.

Which of the following do you mean:
1) If using a spinlock in client_releasepage() is only for mount/umount,
  this implementation is not wise.
2) There is the fact that a spinlock is necessary for blkdev_releasepage().
This fact prevents us from making various implementations of
client_releasepage().
(Without a spinlock, we can implement a client_releasepage() which can release
the buffers with a sleep. As a result, it may enable more buffers release than
before.)

There is the fact that a filesystem can be mounted on several places,
and the lock mechanism is absolutely necessary for this fact.

I also think we are sad that we cannot implement various implementations for
client_releasepage(). But now I cannot imagine what to do for
a client_releasepage() which can sleep, too...

Regards,
Toshiyuki Okajima

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html