| Message ID | 20211110180021.20876-2-khiremat@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: Fix incorrect statfs report | expand |
On Wed, 2021-11-10 at 23:30 +0530, khiremat@redhat.com wrote: > From: Kotresh HR <khiremat@redhat.com> > > Problem: > The statfs reports incorrect free/available space > for quota less then CEPH_BLOCK size (4M). > > Solution: > For quota less than CEPH_BLOCK size, smaller block > size of 4K is used. But if quota is less than 4K, > it is decided to go with binary use/free of 4K > block. For quota size less than 4K size, report the > total=used=4K,free=0 when quota is full and > total=free=4K,used=0 otherwise. > > Signed-off-by: Kotresh HR <khiremat@redhat.com> > --- > fs/ceph/quota.c | 14 ++++++++++++++ > fs/ceph/super.h | 1 + > 2 files changed, 15 insertions(+) > > diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c > index 620c691af40e..24ae13ea2241 100644 > --- a/fs/ceph/quota.c > +++ b/fs/ceph/quota.c > @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > if (ci->i_max_bytes) { > total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; > used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; > + /* For quota size less than 4MB, use 4KB block size */ > + if (!total) { > + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; > + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > + } > /* It is possible for a quota to be exceeded. > * Report 'zero' in that case > */ > free = total > used ? total - used : 0; > + /* For quota size less than 4KB, report the > + * total=used=4KB,free=0 when quota is full > + * and total=free=4KB, used=0 otherwise */ > + if (!total) { > + total = 1; > + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > + } We really ought to have the MDS establish a floor for the quota size. <4k quota seems ridiculous. > } > spin_unlock(&ci->i_ceph_lock); > if (total) { > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index ed51e04739c4..387ee33894db 100644 > --- a/fs/ceph/super.h > +++ b/fs/ceph/super.h > @@ -32,6 +32,7 @@ > * large volume sizes on 32-bit machines. */ > #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ > #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) > +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ > > #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ > #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */ This looks good, Kotresh. Nice work. I'll plan to merge this into testing in the near term. Before we merge this into mainline though, it would be good to have a testcase that ensures that this works like we expect with these small quotas. Could you write something for teuthology? Thanks,
On Thu, Nov 11, 2021 at 12:48 AM Jeff Layton <jlayton@kernel.org> wrote: > > On Wed, 2021-11-10 at 23:30 +0530, khiremat@redhat.com wrote: > > From: Kotresh HR <khiremat@redhat.com> > > > > Problem: > > The statfs reports incorrect free/available space > > for quota less then CEPH_BLOCK size (4M). > > > > Solution: > > For quota less than CEPH_BLOCK size, smaller block > > size of 4K is used. But if quota is less than 4K, > > it is decided to go with binary use/free of 4K > > block. For quota size less than 4K size, report the > > total=used=4K,free=0 when quota is full and > > total=free=4K,used=0 otherwise. > > > > Signed-off-by: Kotresh HR <khiremat@redhat.com> > > --- > > fs/ceph/quota.c | 14 ++++++++++++++ > > fs/ceph/super.h | 1 + > > 2 files changed, 15 insertions(+) > > > > diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c > > index 620c691af40e..24ae13ea2241 100644 > > --- a/fs/ceph/quota.c > > +++ b/fs/ceph/quota.c > > @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > > if (ci->i_max_bytes) { > > total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; > > used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; > > + /* For quota size less than 4MB, use 4KB block size */ > > + if (!total) { > > + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; > > + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; > > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > > + } > > /* It is possible for a quota to be exceeded. > > * Report 'zero' in that case > > */ > > free = total > used ? total - used : 0; > > + /* For quota size less than 4KB, report the > > + * total=used=4KB,free=0 when quota is full > > + * and total=free=4KB, used=0 otherwise */ > > + if (!total) { > > + total = 1; > > + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; > > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > > + } > > We really ought to have the MDS establish a floor for the quota size. > <4k quota seems ridiculous. Yes, I have opened a tracker for the same. https://tracker.ceph.com/issues/53228 > > > > } > > spin_unlock(&ci->i_ceph_lock); > > if (total) { > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > index ed51e04739c4..387ee33894db 100644 > > --- a/fs/ceph/super.h > > +++ b/fs/ceph/super.h > > @@ -32,6 +32,7 @@ > > * large volume sizes on 32-bit machines. */ > > #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ > > #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) > > +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ > > > > #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ > > #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */ > > This looks good, Kotresh. Nice work. I'll plan to merge this into > testing in the near term. > > Before we merge this into mainline though, it would be good to have a > testcase that ensures that this works like we expect with these small > quotas. > > Could you write something for teuthology? Sure, I will add a teuthology test for this. > > > Thanks, > -- > Jeff Layton <jlayton@kernel.org> > -- Thanks, Kotresh HR
On 11/11/21 2:00 AM, khiremat@redhat.com wrote: > From: Kotresh HR <khiremat@redhat.com> > > Problem: > The statfs reports incorrect free/available space > for quota less then CEPH_BLOCK size (4M). s/then/than/ > Solution: > For quota less than CEPH_BLOCK size, smaller block > size of 4K is used. But if quota is less than 4K, > it is decided to go with binary use/free of 4K > block. For quota size less than 4K size, report the > total=used=4K,free=0 when quota is full and > total=free=4K,used=0 otherwise. > > Signed-off-by: Kotresh HR <khiremat@redhat.com> > --- > fs/ceph/quota.c | 14 ++++++++++++++ > fs/ceph/super.h | 1 + > 2 files changed, 15 insertions(+) > > diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c > index 620c691af40e..24ae13ea2241 100644 > --- a/fs/ceph/quota.c > +++ b/fs/ceph/quota.c > @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > if (ci->i_max_bytes) { > total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; > used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; > + /* For quota size less than 4MB, use 4KB block size */ > + if (!total) { > + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; > + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > + } > /* It is possible for a quota to be exceeded. > * Report 'zero' in that case > */ > free = total > used ? total - used : 0; > + /* For quota size less than 4KB, report the > + * total=used=4KB,free=0 when quota is full > + * and total=free=4KB, used=0 otherwise */ > + if (!total) { > + total = 1; > + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; The 'buf->f_frsize' has already been assigned above, this could be removed. Thanks -- Xiubo > + } > } > spin_unlock(&ci->i_ceph_lock); > if (total) { > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index ed51e04739c4..387ee33894db 100644 > --- a/fs/ceph/super.h > +++ b/fs/ceph/super.h > @@ -32,6 +32,7 @@ > * large volume sizes on 32-bit machines. */ > #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ > #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) > +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ > > #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ > #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */
On Mon, Nov 15, 2021 at 8:24 AM Xiubo Li <xiubli@redhat.com> wrote: > > > On 11/11/21 2:00 AM, khiremat@redhat.com wrote: > > From: Kotresh HR <khiremat@redhat.com> > > > > Problem: > > The statfs reports incorrect free/available space > > for quota less then CEPH_BLOCK size (4M). > > s/then/than/ > > > > Solution: > > For quota less than CEPH_BLOCK size, smaller block > > size of 4K is used. But if quota is less than 4K, > > it is decided to go with binary use/free of 4K > > block. For quota size less than 4K size, report the > > total=used=4K,free=0 when quota is full and > > total=free=4K,used=0 otherwise. > > > > Signed-off-by: Kotresh HR <khiremat@redhat.com> > > --- > > fs/ceph/quota.c | 14 ++++++++++++++ > > fs/ceph/super.h | 1 + > > 2 files changed, 15 insertions(+) > > > > diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c > > index 620c691af40e..24ae13ea2241 100644 > > --- a/fs/ceph/quota.c > > +++ b/fs/ceph/quota.c > > @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > > if (ci->i_max_bytes) { > > total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; > > used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; > > + /* For quota size less than 4MB, use 4KB block size */ > > + if (!total) { > > + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; > > + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; > > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > > + } > > /* It is possible for a quota to be exceeded. > > * Report 'zero' in that case > > */ > > free = total > used ? total - used : 0; > > + /* For quota size less than 4KB, report the > > + * total=used=4KB,free=0 when quota is full > > + * and total=free=4KB, used=0 otherwise */ > > + if (!total) { > > + total = 1; > > + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; > > + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > > The 'buf->f_frsize' has already been assigned above, this could be removed. > If the quota size is less than 4KB, the above assignment is not hit. This is required. > Thanks > > -- Xiubo > > > + } > > } > > spin_unlock(&ci->i_ceph_lock); > > if (total) { > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > index ed51e04739c4..387ee33894db 100644 > > --- a/fs/ceph/super.h > > +++ b/fs/ceph/super.h > > @@ -32,6 +32,7 @@ > > * large volume sizes on 32-bit machines. */ > > #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ > > #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) > > +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ > > > > #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ > > #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */ >
On 11/15/21 1:30 PM, Kotresh Hiremath Ravishankar wrote: > On Mon, Nov 15, 2021 at 8:24 AM Xiubo Li <xiubli@redhat.com> wrote: >> >> On 11/11/21 2:00 AM, khiremat@redhat.com wrote: >>> From: Kotresh HR <khiremat@redhat.com> >>> >>> Problem: >>> The statfs reports incorrect free/available space >>> for quota less then CEPH_BLOCK size (4M). >> s/then/than/ >> >> >>> Solution: >>> For quota less than CEPH_BLOCK size, smaller block >>> size of 4K is used. But if quota is less than 4K, >>> it is decided to go with binary use/free of 4K >>> block. For quota size less than 4K size, report the >>> total=used=4K,free=0 when quota is full and >>> total=free=4K,used=0 otherwise. >>> >>> Signed-off-by: Kotresh HR <khiremat@redhat.com> >>> --- >>> fs/ceph/quota.c | 14 ++++++++++++++ >>> fs/ceph/super.h | 1 + >>> 2 files changed, 15 insertions(+) >>> >>> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c >>> index 620c691af40e..24ae13ea2241 100644 >>> --- a/fs/ceph/quota.c >>> +++ b/fs/ceph/quota.c >>> @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) >>> if (ci->i_max_bytes) { >>> total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; >>> used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; >>> + /* For quota size less than 4MB, use 4KB block size */ >>> + if (!total) { >>> + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; >>> + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; >>> + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; If the quota size is less than 4MB, the 'buf->f_frsize' will always be set here, including less than 4KB. >>> + } >>> /* It is possible for a quota to be exceeded. >>> * Report 'zero' in that case >>> */ >>> free = total > used ? total - used : 0; >>> + /* For quota size less than 4KB, report the >>> + * total=used=4KB,free=0 when quota is full >>> + * and total=free=4KB, used=0 otherwise */ >>> + if (!total) { >>> + total = 1; >>> + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; >>> + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; >> The 'buf->f_frsize' has already been assigned above, this could be removed. >> > If the quota size is less than 4KB, the above assignment is not hit. > This is required. > >> Thanks >> >> -- Xiubo >> >>> + } >>> } >>> spin_unlock(&ci->i_ceph_lock); >>> if (total) { >>> diff --git a/fs/ceph/super.h b/fs/ceph/super.h >>> index ed51e04739c4..387ee33894db 100644 >>> --- a/fs/ceph/super.h >>> +++ b/fs/ceph/super.h >>> @@ -32,6 +32,7 @@ >>> * large volume sizes on 32-bit machines. */ >>> #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ >>> #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) >>> +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ >>> >>> #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ >>> #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */ >
On Mon, Nov 15, 2021 at 11:12 AM Xiubo Li <xiubli@redhat.com> wrote: > > > On 11/15/21 1:30 PM, Kotresh Hiremath Ravishankar wrote: > > On Mon, Nov 15, 2021 at 8:24 AM Xiubo Li <xiubli@redhat.com> wrote: > >> > >> On 11/11/21 2:00 AM, khiremat@redhat.com wrote: > >>> From: Kotresh HR <khiremat@redhat.com> > >>> > >>> Problem: > >>> The statfs reports incorrect free/available space > >>> for quota less then CEPH_BLOCK size (4M). > >> s/then/than/ > >> > >> > >>> Solution: > >>> For quota less than CEPH_BLOCK size, smaller block > >>> size of 4K is used. But if quota is less than 4K, > >>> it is decided to go with binary use/free of 4K > >>> block. For quota size less than 4K size, report the > >>> total=used=4K,free=0 when quota is full and > >>> total=free=4K,used=0 otherwise. > >>> > >>> Signed-off-by: Kotresh HR <khiremat@redhat.com> > >>> --- > >>> fs/ceph/quota.c | 14 ++++++++++++++ > >>> fs/ceph/super.h | 1 + > >>> 2 files changed, 15 insertions(+) > >>> > >>> diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c > >>> index 620c691af40e..24ae13ea2241 100644 > >>> --- a/fs/ceph/quota.c > >>> +++ b/fs/ceph/quota.c > >>> @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) > >>> if (ci->i_max_bytes) { > >>> total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; > >>> used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; > >>> + /* For quota size less than 4MB, use 4KB block size */ > >>> + if (!total) { > >>> + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; > >>> + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; > >>> + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > > If the quota size is less than 4MB, the 'buf->f_frsize' will always be > set here, including less than 4KB. Yes, sorry for the confusion. I will address this. > > > >>> + } > >>> /* It is possible for a quota to be exceeded. > >>> * Report 'zero' in that case > >>> */ > >>> free = total > used ? total - used : 0; > >>> + /* For quota size less than 4KB, report the > >>> + * total=used=4KB,free=0 when quota is full > >>> + * and total=free=4KB, used=0 otherwise */ > >>> + if (!total) { > >>> + total = 1; > >>> + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; > >>> + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; > >> The 'buf->f_frsize' has already been assigned above, this could be removed. > >> > > If the quota size is less than 4KB, the above assignment is not hit. > > This is required. > > > >> Thanks > >> > >> -- Xiubo > >> > >>> + } > >>> } > >>> spin_unlock(&ci->i_ceph_lock); > >>> if (total) { > >>> diff --git a/fs/ceph/super.h b/fs/ceph/super.h > >>> index ed51e04739c4..387ee33894db 100644 > >>> --- a/fs/ceph/super.h > >>> +++ b/fs/ceph/super.h > >>> @@ -32,6 +32,7 @@ > >>> * large volume sizes on 32-bit machines. */ > >>> #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ > >>> #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) > >>> +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ > >>> > >>> #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ > >>> #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */ > > >
diff --git a/fs/ceph/quota.c b/fs/ceph/quota.c index 620c691af40e..24ae13ea2241 100644 --- a/fs/ceph/quota.c +++ b/fs/ceph/quota.c @@ -494,10 +494,24 @@ bool ceph_quota_update_statfs(struct ceph_fs_client *fsc, struct kstatfs *buf) if (ci->i_max_bytes) { total = ci->i_max_bytes >> CEPH_BLOCK_SHIFT; used = ci->i_rbytes >> CEPH_BLOCK_SHIFT; + /* For quota size less than 4MB, use 4KB block size */ + if (!total) { + total = ci->i_max_bytes >> CEPH_4K_BLOCK_SHIFT; + used = ci->i_rbytes >> CEPH_4K_BLOCK_SHIFT; + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; + } /* It is possible for a quota to be exceeded. * Report 'zero' in that case */ free = total > used ? total - used : 0; + /* For quota size less than 4KB, report the + * total=used=4KB,free=0 when quota is full + * and total=free=4KB, used=0 otherwise */ + if (!total) { + total = 1; + free = ci->i_max_bytes > ci->i_rbytes ? 1 : 0; + buf->f_frsize = 1 << CEPH_4K_BLOCK_SHIFT; + } } spin_unlock(&ci->i_ceph_lock); if (total) { diff --git a/fs/ceph/super.h b/fs/ceph/super.h index ed51e04739c4..387ee33894db 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -32,6 +32,7 @@ * large volume sizes on 32-bit machines. */ #define CEPH_BLOCK_SHIFT 22 /* 4 MB */ #define CEPH_BLOCK (1 << CEPH_BLOCK_SHIFT) +#define CEPH_4K_BLOCK_SHIFT 12 /* 4 KB */ #define CEPH_MOUNT_OPT_CLEANRECOVER (1<<1) /* auto reonnect (clean mode) after blocklisted */ #define CEPH_MOUNT_OPT_DIRSTAT (1<<4) /* `cat dirname` for stats */