| Message ID | 1438235311-23788-24-git-send-email-yangds.fnst@cn.fujitsu.com |
|---|---|
| State | Superseded |
| Headers | show |
Am 30.07.2015 um 07:48 schrieb Dongsheng Yang:
> We need to set or clear MS_RDONLY in remounting.
Care to explain why?
Does the quota subsystem need that information?
If so, where?
Thanks,
//richard
On 08/09/2015 05:17 AM, Richard Weinberger wrote: > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: >> We need to set or clear MS_RDONLY in remounting. > > Care to explain why? > Does the quota subsystem need that information? > If so, where? Ha, yes, you are right. When we remount to rw from ro, we need to call dquot_resume. And dquot_resume will call vfs_load_quota_inode() which will check the MS_RDONLY. If it's ro mode, will return an -EROFS. And I know that we are using ->ro_mount in ubifs to store the mounted mode. So we don't care about the MS_RDONLY in ubifs. But why not to tell vfs we are in ro or rw? Does it cause any problem? Thanx YAng > Thanks, > //richard > . >
On 08/10/2015 10:46 AM, Dongsheng Yang wrote: > On 08/09/2015 05:17 AM, Richard Weinberger wrote: >> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: >>> We need to set or clear MS_RDONLY in remounting. >> >> Care to explain why? >> Does the quota subsystem need that information? >> If so, where? > > Ha, yes, you are right. > > When we remount to rw from ro, we need to call dquot_resume. > And dquot_resume will call vfs_load_quota_inode() which > will check the MS_RDONLY. If it's ro mode, will return > an -EROFS. > > And I know that we are using ->ro_mount in ubifs > to store the mounted mode. So we don't care about > the MS_RDONLY in ubifs. But why not to tell vfs we are in > ro or rw? Does it cause any problem? Hi Artem, I found there is a commit 2ef13294d to introduce ro_mount to ubifs. Yes, I see the reason to use ro_mount rather than MS_RDONLY in ubifs. But why we do not set MS_RDONLY to tell vfs it's in rdonly or not? Current quota depends on it. Does this patch cause some problem? Yang > > Thanx > YAng > >> Thanks, >> //richard >> . >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > . >
On Mon, 2015-08-24 at 09:29 +0800, Dongsheng Yang wrote: > On 08/10/2015 10:46 AM, Dongsheng Yang wrote: > > On 08/09/2015 05:17 AM, Richard Weinberger wrote: > > > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: > > > > We need to set or clear MS_RDONLY in remounting. > > > > > > Care to explain why? > > > Does the quota subsystem need that information? > > > If so, where? > > > > Ha, yes, you are right. > > > > When we remount to rw from ro, we need to call dquot_resume. > > And dquot_resume will call vfs_load_quota_inode() which > > will check the MS_RDONLY. If it's ro mode, will return > > an -EROFS. > > > > And I know that we are using ->ro_mount in ubifs > > to store the mounted mode. So we don't care about > > the MS_RDONLY in ubifs. But why not to tell vfs we are in > > ro or rw? Does it cause any problem? > > Hi Artem, > I found there is a commit 2ef13294d to introduce > ro_mount to ubifs. Yes, I see the reason to use ro_mount > rather than MS_RDONLY in ubifs. But why we do not set > MS_RDONLY to tell vfs it's in rdonly or not? Current > quota depends on it. Does this patch cause some problem? MS_RDONLY is set at mount time, then it can be changed when we re -mount, and then UBIFS can change it when there was a critical error, to ask VFS to stop writing more data to the file-system. See 'ubifs_ro_mode()'. The custom ro_mount is the same as MS_RDONLY, but it does not change in 'ubifs_ro_mode()'. It preserves the last mount state before the error happened - was it R/O or R/W. Why is this needed? UBIFS allocate different amount of resources depending on whether we are R/O or R/W. We use less memory in R/O mode, for example. At unmount time, we need to know whether we allocated the "RW" amount of resources or the "RO" amount of resources. We cannot use the MS_RDONLY flag to detect that, because it is changed in 'ubifs_ro_mode()' unconditionally, so we lose the information. Therefore we use the custom ro_mount flag - it tells us how we were mounted, so we know how much resources to free. Artem.
On 08/24/2015 03:02 PM, Artem Bityutskiy wrote: > On Mon, 2015-08-24 at 09:29 +0800, Dongsheng Yang wrote: >> On 08/10/2015 10:46 AM, Dongsheng Yang wrote: >>> On 08/09/2015 05:17 AM, Richard Weinberger wrote: >>>> Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: >>>>> We need to set or clear MS_RDONLY in remounting. >>>> >>>> Care to explain why? >>>> Does the quota subsystem need that information? >>>> If so, where? >>> >>> Ha, yes, you are right. >>> >>> When we remount to rw from ro, we need to call dquot_resume. >>> And dquot_resume will call vfs_load_quota_inode() which >>> will check the MS_RDONLY. If it's ro mode, will return >>> an -EROFS. >>> >>> And I know that we are using ->ro_mount in ubifs >>> to store the mounted mode. So we don't care about >>> the MS_RDONLY in ubifs. But why not to tell vfs we are in >>> ro or rw? Does it cause any problem? >> >> Hi Artem, >> I found there is a commit 2ef13294d to introduce >> ro_mount to ubifs. Yes, I see the reason to use ro_mount >> rather than MS_RDONLY in ubifs. But why we do not set >> MS_RDONLY to tell vfs it's in rdonly or not? Current >> quota depends on it. Does this patch cause some problem? > > MS_RDONLY is set at mount time, then it can be changed when we re > -mount, and then UBIFS can change it when there was a critical error, > to ask VFS to stop writing more data to the file-system. See > 'ubifs_ro_mode()'. > > The custom ro_mount is the same as MS_RDONLY, but it does not change in > 'ubifs_ro_mode()'. It preserves the last mount state before the error > happened - was it R/O or R/W. > > Why is this needed? > > UBIFS allocate different amount of resources depending on whether we > are R/O or R/W. We use less memory in R/O mode, for example. > > At unmount time, we need to know whether we allocated the "RW" amount > of resources or the "RO" amount of resources. > > We cannot use the MS_RDONLY flag to detect that, because it is changed > in 'ubifs_ro_mode()' unconditionally, so we lose the information. > Therefore we use the custom ro_mount flag - it tells us how we were > mounted, so we know how much resources to free. Great explanation!!! This makes it much more clear to me. But about: > MS_RDONLY is set at mount time, then it can be changed when we re Currently, ubifs did not set it at mount time and ubifs did not change it in re-mounting. So vfs don't know the R/O or R/W mode of ubifs. That's the reason I sent this patch to set the MS_RDONLY in ubifs properly. I was concerned that will it cause some problem. But now, from you explanation, this patch is a correct fix I think. Thanx Yang > > Artem. > . >
On Mon, 2015-08-24 at 15:12 +0800, Dongsheng Yang wrote: > On 08/24/2015 03:02 PM, Artem Bityutskiy wrote: > > On Mon, 2015-08-24 at 09:29 +0800, Dongsheng Yang wrote: > > > On 08/10/2015 10:46 AM, Dongsheng Yang wrote: > > > > On 08/09/2015 05:17 AM, Richard Weinberger wrote: > > > > > Am 30.07.2015 um 07:48 schrieb Dongsheng Yang: > > > > > > We need to set or clear MS_RDONLY in remounting. > > > > > > > > > > Care to explain why? > > > > > Does the quota subsystem need that information? > > > > > If so, where? > > > > > > > > Ha, yes, you are right. > > > > > > > > When we remount to rw from ro, we need to call dquot_resume. > > > > And dquot_resume will call vfs_load_quota_inode() which > > > > will check the MS_RDONLY. If it's ro mode, will return > > > > an -EROFS. > > > > > > > > And I know that we are using ->ro_mount in ubifs > > > > to store the mounted mode. So we don't care about > > > > the MS_RDONLY in ubifs. But why not to tell vfs we are in > > > > ro or rw? Does it cause any problem? > > > > > > Hi Artem, > > > I found there is a commit 2ef13294d to introduce > > > ro_mount to ubifs. Yes, I see the reason to use ro_mount > > > rather than MS_RDONLY in ubifs. But why we do not set > > > MS_RDONLY to tell vfs it's in rdonly or not? Current > > > quota depends on it. Does this patch cause some problem? > > > > MS_RDONLY is set at mount time, then it can be changed when we re > > -mount, and then UBIFS can change it when there was a critical > > error, > > to ask VFS to stop writing more data to the file-system. See > > 'ubifs_ro_mode()'. > > > > The custom ro_mount is the same as MS_RDONLY, but it does not > > change in > > 'ubifs_ro_mode()'. It preserves the last mount state before the > > error > > happened - was it R/O or R/W. > > > > Why is this needed? > > > > UBIFS allocate different amount of resources depending on whether > > we > > are R/O or R/W. We use less memory in R/O mode, for example. > > > > At unmount time, we need to know whether we allocated the "RW" > > amount > > of resources or the "RO" amount of resources. > > > > We cannot use the MS_RDONLY flag to detect that, because it is > > changed > > in 'ubifs_ro_mode()' unconditionally, so we lose the information. > > Therefore we use the custom ro_mount flag - it tells us how we were > > mounted, so we know how much resources to free. > > Great explanation!!! This makes it much more clear to me. > > But about: > > MS_RDONLY is set at mount time, then it can be changed when we re > > Currently, ubifs did not set it at mount time and ubifs did not > change > it in re-mounting. So vfs don't know the R/O or R/W mode of ubifs. Well, sounds like a bug. Either we missed that, or VFS used to set it, and now does not. In either case, IIUC, the MS_RDONLY flag should be set on remount and reflect the mount state. Please, verify /proc/mounts after RO<->RW remounts - we must make sure mount options are correct there. You can cook a patch and send it. Do not forget to add the stable tag then. Artem.
On 08/24/2015 03:26 PM, Artem Bityutskiy wrote: > On Mon, 2015-08-24 at 15:12 +0800, Dongsheng Yang wrote: >> On 08/24/2015 03:02 PM, Artem Bityutskiy wrote: [...] > > Well, sounds like a bug. Either we missed that, or VFS used to set it, Hi Artem, Yes, VFS is setting flags after sb->s_op->remount_fs(). But someone would use it in remount_fs() such as quota. That's the reason for this patch. What I want here is to make our ubifs working similarly with other filesystems setting or clearing MS_RDONLY in remount_fs(). Because someone in vfs needs this information. Although that's another topic, we set MS_RDONLY twice, one in ubifs, two in VFS. But they are different. in ubifs, we only set the MS_RDONLY in remount_ro() specially for MS_RDONLY. But in VFS, we are setting all flags for s_flags in remounting. So, I think it's okey in design. > and now does not. In either case, IIUC, the MS_RDONLY flag should be > set on remount and reflect the mount state. Please, verify /proc/mounts > after RO<->RW remounts - we must make sure mount options are correct > there. You can cook a patch and send it. Do not forget to add the > stable tag then. Before adding quota for ubifs, there is no user to use MS_RDONLY before do_remount() returning. So there is no problem before adding quota in ubifs and no need for stable then. Thanx Yang > > Artem. > . >
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 9fdfd69..6ed35e2 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -1953,6 +1953,7 @@ static int ubifs_remount_rw(struct ubifs_info *c) */ err = dbg_check_space_info(c); } + c->vfs_sb->s_flags &= ~MS_RDONLY; mutex_unlock(&c->umount_mutex); return err; @@ -2019,6 +2020,7 @@ static void ubifs_remount_ro(struct ubifs_info *c) err = dbg_check_space_info(c); if (err) ubifs_ro_mode(c, err); + c->vfs_sb->s_flags |= MS_RDONLY; mutex_unlock(&c->umount_mutex); }
We need to set or clear MS_RDONLY in remounting. Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com> --- fs/ubifs/super.c | 2 ++ 1 file changed, 2 insertions(+)