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

[Hui Wang <hui.wang@canonical.com>]

On 07/25/2013 06:03 PM, Jan Kara wrote:
> 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
Sounds a better idea. Right now, if we mount the DVD-R without readonly, 
the mount will
try RW mount first, but it will fail since the media is readonly, then 
the mount(8) command will
try again with MS_RDONLY. So your design will work similarly to this 
case, the difference is
filesystem is readonly, the first time mounting will fail from fs.

Hope to see your patch soon. :-)

Regards,
Hui.