[Cluster-devel] [PATCH] Move struct gfs2_rgrp_lvb out of gfs2_ondisk.h

[Andrew Price]

On 15/01/2020 08:58, Steven Whitehouse wrote:
> Hi,
> 
> On 15/01/2020 08:49, Andreas Gruenbacher wrote:
>> There's no point in sharing the internal structure of lock value blocks
>> with user space.
> 
> The reason that is in ondisk is that changing that structure is 
> something that needs to follow the same rules as changing the on disk 
> structures. So it is there as a reminder of that,

Perhaps some eye-catching code comments would suffice? Defining it in a 
uapi header does sort-of communicate that it's ok to use in userspace, 
whereas just having the structs in close proximity doesn't really 
communicate that they should be updated in sync.

Andy

> 
>>
>> Signed-off-by: Andreas Gruenbacher <agruenba at redhat.com>
>> ---
>>   fs/gfs2/glock.h                  |  1 +
>>   fs/gfs2/incore.h                 |  1 +
>>   fs/gfs2/rgrp.c                   | 10 ++++++++++
>>   include/uapi/linux/gfs2_ondisk.h | 10 ----------
>>   4 files changed, 12 insertions(+), 10 deletions(-)
>>

> 
> Steve.
> 
>> diff --git a/fs/gfs2/glock.h b/fs/gfs2/glock.h
>> index b8adaf80e4c5..d2f2dba05a94 100644
>> --- a/fs/gfs2/glock.h
>> +++ b/fs/gfs2/glock.h
>> @@ -306,4 +306,5 @@ static inline void glock_clear_object(struct 
>> gfs2_glock *gl, void *object)
>>       spin_unlock(&gl->gl_lockref.lock);
>>   }
>> +
>>   #endif /* __GLOCK_DOT_H__ */
>> diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
>> index b5d9c11f4901..5155389e9b5c 100644
>> --- a/fs/gfs2/incore.h
>> +++ b/fs/gfs2/incore.h
>> @@ -33,6 +33,7 @@ struct gfs2_trans;
>>   struct gfs2_jdesc;
>>   struct gfs2_sbd;
>>   struct lm_lockops;
>> +struct gfs2_rgrp_lvb;
>>   typedef void (*gfs2_glop_bh_t) (struct gfs2_glock *gl, unsigned int 
>> ret);
>> diff --git a/fs/gfs2/rgrp.c b/fs/gfs2/rgrp.c
>> index 2466bb44a23c..1165627274cf 100644
>> --- a/fs/gfs2/rgrp.c
>> +++ b/fs/gfs2/rgrp.c
>> @@ -46,6 +46,16 @@
>>   #define LBITSKIP00 (0x0000000000000000UL)
>>   #endif
>> +struct gfs2_rgrp_lvb {
>> +    __be32 rl_magic;
>> +    __be32 rl_flags;
>> +    __be32 rl_free;
>> +    __be32 rl_dinodes;
>> +    __be64 rl_igeneration;
>> +    __be32 rl_unlinked;
>> +    __be32 __pad;
>> +};
>> +
>>   /*
>>    * These routines are used by the resource group routines (rgrp.c)
>>    * to keep track of block allocation.  Each block is represented by two
>> diff --git a/include/uapi/linux/gfs2_ondisk.h 
>> b/include/uapi/linux/gfs2_ondisk.h
>> index 2dc10a034de1..4e9a80941bec 100644
>> --- a/include/uapi/linux/gfs2_ondisk.h
>> +++ b/include/uapi/linux/gfs2_ondisk.h
>> @@ -171,16 +171,6 @@ struct gfs2_rindex {
>>   #define GFS2_RGF_NOALLOC    0x00000008
>>   #define GFS2_RGF_TRIMMED    0x00000010
>> -struct gfs2_rgrp_lvb {
>> -    __be32 rl_magic;
>> -    __be32 rl_flags;
>> -    __be32 rl_free;
>> -    __be32 rl_dinodes;
>> -    __be64 rl_igeneration;
>> -    __be32 rl_unlinked;
>> -    __be32 __pad;
>> -};
>> -
>>   struct gfs2_rgrp {
>>       struct gfs2_meta_header rg_header;
>