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

[Jan Kara <jack@suse.cz>]

On Thu 25-07-13 11:24:00, Hui Wang wrote:
> On 07/25/2013 02:39 AM, Jan Kara wrote:
> >On Wed 24-07-13 18:08:40, Hui Wang wrote:
> >>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
> [...]
> >>>>>     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.
> >   I see. For iso9660 filesystem it is actually pretty easy to fix since
> >there we *know* we are going to mount it read only (isofs_mount() could
> >well add S_RDONLY to the flags passed to mount_bdev() which would fix the
> >issue). With udf it is more complex as there we could mount it read-write
> >depending on the device, medium, and filesystem features. So there we
> >really need to be able to drop write access as you suggested. But maybe I'd
> >put your code into some function like bdev_drop_write_access() in
> >fs/block_dev.c so that we don't leak bdev peculiarities into superblock
> >handling functions.
> >
> Your analysis is reasonable, the iso9660 is easier to be handled than udf.
> 
> I also think move the relating code to fs/block_dev.c is a good idea, and
> i am very glad this issue can raise your attention, looking forward a
> perfect solution for this issue from you.
  Actually, I've now got idea that seems technically even better to me.
When we are asked to RW mount in case filesystem cannot really do RW, we
return -EROFS instead of silently falling back to RO mount. Userspace
mount(8) command will try again with MS_RDONLY set so things should still
work OK and the problems with eject button would be solved. I'll try to
code that up.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR