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
