[Cluster-devel] [PATCH] libgfs2: Import gfs2_ondisk.h

[Andrew Price]

On 09/04/2019 14:03, Steven Whitehouse wrote:
> On 09/04/2019 13:48, Andrew Price wrote:
>> On 09/04/2019 13:21, Steven Whitehouse wrote:
>>> Those conversion functions are not sensible, thats why we got rid of 
>>> them from the kernel code. 
>>
>> Is it the functions that aren't sensible or the use of the 
>> gfs2_ondisk.h structs as the containers for the native endian data? 
>> I'm not sure I get why the kernel functions like gfs2_dinode_in() are 
>> considered sensible and gfs2-utils' gfs2_dinode_in(), which does a 
>> similar thing but with a different struct, isn't sensible.
>>
> Well in general we don't want to convert lots of fields in what is 
> basically a copy. The inode, when it is read in is an exception to that 
> mainly because we have to in order to make sure that the vfs level data 
> is all up to date. Keeping the structs as containers is useful, so yes 
> we want to retain that. In many cases though we only need a few fields 
> from what can be quite large data structures, so in those cases we 
> should read/update the fields that we care about for that particular 
> operation, rather than converting the whole data structure each time. We 
> got a fair speed up when we made that change in the kernel.
> 
> So generally I'd like to discourage the blanket conversion functions, 
> though it is likely we'll need to retain a few of them, in favour of 
> converting just the required fields at the point of use. This should be 
> safe to do given that we have the ability to do compile time type 
> checking - and lets try and include that in the tests that are always 
> run before check in, to make sure that we don't land up with any 
> mistakes. That would be a good addition to the tests I think,

Ah ok, that makes sense to me, thanks for explaining. I'm sure we could 
speed up bits of gfs2-utils by taking that approach too.

Andy