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
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
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
