On 2014-01-24 05:56, Ryusuke Konishi wrote: > On Tue, 21 Jan 2014 15:00:28 +0100, Andreas Rohner wrote: >> This adds struct nilfs_suinfo_update and some flags. It contains the >> information needed to update one entry of segment usage information in >> the SUFILE. The flags specify, which fields need to be updated. >> >> This can be used to update segment usage information from userspace with an >> ioctl. >> >> Signed-off-by: Andreas Rohner <[email protected]> >> --- >> include/linux/nilfs2_fs.h | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h >> index 9875576..2ee1594 100644 >> --- a/include/linux/nilfs2_fs.h >> +++ b/include/linux/nilfs2_fs.h >> @@ -709,6 +709,45 @@ static inline int nilfs_suinfo_clean(const struct >> nilfs_suinfo *si) >> return !si->sui_flags; >> } >> >> +/** >> + * nilfs_suinfo_update - segment usage information update >> + * @sup_segnum: segment number >> + * @sup_flags: flags for which fields are active in sup_sui >> + * @sup_sui: segment usage information >> + */ >> +struct nilfs_suinfo_update { >> + __u64 sup_segnum; >> + __u32 sup_flags; >> + struct nilfs_suinfo sup_sui; >> +}; > > This structure is not aligned on 8 byte boundary. You need a 4 byte > pad to make the alignment architecture independent since the structure > already has some 64-bit variables. > > If you do not so, the ioctl will lose compatibility, for instance, > between 32 bit userland programs and 64 bit kernel. > > struct nilfs_suinfo_update { > __u64 sup_segnum; > __u32 sup_flags; > __u32 sup_reserved; > struct nilfs_suinfo sup_sui; > };
Ok, I thought 4 byte alignment would be enough. But now I see the problem, nilfs_suinfo contains 64 bit values, that need to be 8 byte aligned on a 64 bit architecture. > Do you really need different suinfo update flags per segment ? No. > If it's not so, we don't have to add nilfs_suinfo_update structure. > v_flags of nilfs_argv structure looks to be available for that purpose. Yes v_flags is available, but I need nilfs_suinfo_update to contain the segment number. I can't use v_index for the segment number like with GET_SUINFO, because the segment numbers are not necessarily sequential. If I use v_index I could effectively only use the ioctl to update one segment at a time. So I thought, since I need nilfs_suinfo_update anyway, I might as well put the flags there. But with the extra alignment it gets a bit ugly, so I tend to agree with you to put the flags into v_flags now. Maybe I missed something, but how would you propose to send the segment numbers, only using nilfs_suinfo and nilfs_argv? It is certainly possible to use v_index of nilfs_argv and update one segment at a time. It shouldn't be a problem, because there are at most #define NILFS_CLDCONFIG_NSEGMENTS_PER_CLEAN_MAX 32 segments to be updated. > It's just a confirmation. Basically, I think this extension > is acceptable. Cool. Thanks! br, Andreas Rohner -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
