Re: [PATCH] vfs: try to unblock evpoll if mounted filesystem is RDONLY

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/24/2013 05:48 AM, Jan Kara wrote:
On Tue 23-07-13 11:45:55, Tejun Heo wrote:
Hello,

(cc'ing Jan and quoting the whole body for him)

On Mon, Jul 08, 2013 at 10:02:54AM +0800, Hui Wang wrote:
When inserting a rw optical disc like a DVD/CD rw disc, and we mount
it without an explicit ro option, the vfs will block its event poll
workqueue to protect it from damaging while writing to disc, the direct
result of the blocking of event poll is to make the eject button can't
work.

This protection is reasonable when the filesystem on the rw disc is
also rw. but if the filessytem on the rw disc is ro, e.g. the iso9660
and udf readonly partition, this protection is a little bit weird and
unneeded, since most people are going to be curious why the eject
button can't work while the mount is ro?

To make the eject button work again while the mounted filesystem is ro,
we should inspect the flags of the filesytem's sb and unblock the evpoll
conditionally, the code refers to the blkdev_put() in the
fs/block_dev.c.

Signed-off-by: Hui Wang <jason77.wang@xxxxxxxxx>
---
I personally don't know if this is a real defect or not, but this
issue is reported by a customer of our company, he said from the user
experience, this is a defect, since no matter the disc is ro or rw,
the mount is ro, the eject button should work.

so far, all DVD/CD and DVD-R/CD-R follow this rule (mount is ro, eject
button can work), but DVD/CD rw discs don't, no matter the mount is ro
or rw, the eject button always can't work. So our finial goal is to make
the eject button can work while the filesystem on the rw disc is ro and
the whole mounting is ro.

  fs/super.c | 19 +++++++++++++++++++
  1 file changed, 19 insertions(+)

diff --git a/fs/super.c b/fs/super.c
index 7465d43..7980602 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1011,6 +1011,25 @@ struct dentry *mount_bdev(struct file_system_type *fs_type,
s->s_flags |= MS_ACTIVE;
  		bdev->bd_super = s;
+
+		mutex_lock(&bdev->bd_mutex);
+
+		if ((s->s_flags & MS_RDONLY) && bdev->bd_write_holder) {
+			int bd_holders;
+
+			bd_holders = bdev->bd_holders;
+			if (bdev == bdev->bd_contains)
+				bd_holders -= 2;
+			else
+				bd_holders -= 1;
+
+			if (!bd_holders) {
+				disk_unblock_events(bdev->bd_disk);
+				bdev->bd_write_holder = false;
+			}
+		}
+
+		mutex_unlock(&bdev->bd_mutex);
While the issue seems legitimate to me, the above seems rather scary
to me.  Can't iso9660 and udf just open the device ro when they know
that's all they need?
   They do that - or better VFS does (see fs/super.c:mount_bdev()). So the
question really is why bd_write_holder gets set. Maybe it didn't get
cleared from previous RW mount because bd_holders never hit zero? Hui have
you found a reason for that?

								Honza
Hi Honza,

The bd_write_holder is set in the blkdev_get() of fs/block_dev.c.
It is intentionally set during the rw optical disc mount process.
Let's make an example to simulate a rw optical disc mount process:

users insert a DVD-RW disc and execute:
"mount -t iso9660 /dev/sr0 /mnt/sr0" in the shell
   |
sys_mount(..., flags, ...) /* with the flag without MS_RDONLY */
   |
do_mount(..., flags, ...)
   |
mount_fs(..., flags, ...)
   |
isofs_mount(..., flags, ...)
   |
mount_bdev(..., flags, ...)
Here the flags is still without MS_RDONLY, and following code
is picked from mount_bdev() and is very important for this
issue:

    if (!(flags & MS_RDONLY))
        mode |= FMODE_WRITE;

    bdev = blkdev_get_by_path(dev_name, mode, fs_type);
                      |
                blkdev_get(..., mode, ...)
Since mode is FMODE_WRITE and rw optical disc is a writble block
device, the bd_write_holder will be set in this function.

The the mount process will go on after that, the mount_bdev() will
continue to call:

s = sget(fs_type, test_bdev_super, set_bdev_super, flags | MS_NOSEC,
         bdev);
the super_block got from the iso9660 filesystem is absolutely
MS_RDONLY, so this mount changes to a readonly mount, but no code to
change the bd_write_holder back to zero and unblock the event poll.


Accutally this problem is very easily to be reproduced, if you use
any linux distribution with kernel equals or is above linux-3.0, you
can insert a DVD-RW/CD-RW which has a iso9660 filesystem, the desktop
auto-mounter daemon will automatically mount this disc, you can see
this mount is readonly through executing mount command in the
terminal, then you can push the eject button on the disc tray, you
will see that button doesn't work anymore.


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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux