On Mon, 2007-02-26 at 14:42 -0700, Andreas Dilger wrote:
> On Feb 25, 2007 02:36 -0800, Andrew Morton wrote:
> > > On Fri, 02 Feb 2007 20:09:40 +0530 Kalpak Shah <[EMAIL PROTECTED]> wrote:
> > > +#define EXT3_INODE_GET_XTIME(xtime, extra_xtime, inode, raw_inode)
> > > \
> > > +do {
> > > \
> > > + (inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime);
> > > \
> > > + \
> > > + if (offsetof(typeof(*raw_inode), extra_xtime) -
> > > \
> > > + offsetof(typeof(*raw_inode), i_extra_isize) +
> > > \
> > > + sizeof((raw_inode)->extra_xtime) <=
> > > \
> > > + le16_to_cpu((raw_inode)->i_extra_isize)) {
> > > \
> > > + if (sizeof((inode)->xtime.tv_sec) > 4) \
> > > + (inode)->xtime.tv_sec |= \
> > > + (__u64)(le32_to_cpu((raw_inode)->extra_xtime) & \
> > > + EXT3_EPOCH_MASK) << 32; \
> > > + (inode)->xtime.tv_nsec = \
> > > + (le32_to_cpu((raw_inode)->extra_xtime) & \
> > > + EXT3_NSEC_MASK) >> 2; \
> > > + }
> > > \
> > > +} while (0)
> >
> > ow, my eyes. Can we find a way to do this in C rather than in cpp?
>
> The macro is working on the field names of the inode in most places
> (e.g. i_mtime, i_ctime, etc) rather than the values. You _could_ do it
> in C, but it would mean moving all of the "offsetof()" into the caller
> (basically putting the whole macro in-line at the caller) or doing math
> on the pointer addresses at runtime instead of compile time.
>
> It boils down to a check whether the particular nanosecond field is
> inside the reserved space in the inode or not, so it ends up a comparison
> against a constant. For ctime:
>
> if (4 <= le32_to_cpu((raw_inode)->i_extra_isize) {
>
> And the second "if" decides whether to save bits > 32 in the seconds
> field for 64-bit architectures, so it is also evaluated at compile time.
>
> Better to have this in a macro than in the code itself.
I wanted to see if I could clean this up, and this is what I came up
with. I also did some macro magic to make these macros take one less
argument. I ended up breaking two macros up into three smaller macros
and two inline functions. Does this look any better? (This is
incremental against Kalpak's patch.)
Signed-off-by: Dave Kleikamp <[EMAIL PROTECTED]>
diff -Nurp linux.orig/fs/ext3/inode.c linux/fs/ext3/inode.c
--- linux.orig/fs/ext3/inode.c 2007-02-26 17:10:22.000000000 -0600
+++ linux/fs/ext3/inode.c 2007-02-26 17:02:28.000000000 -0600
@@ -2674,10 +2674,10 @@ void ext3_read_inode(struct inode * inod
inode->i_nlink = le16_to_cpu(raw_inode->i_links_count);
inode->i_size = le32_to_cpu(raw_inode->i_size);
- EXT3_INODE_GET_XTIME(i_ctime, i_ctime_extra, inode, raw_inode);
- EXT3_INODE_GET_XTIME(i_mtime, i_mtime_extra, inode, raw_inode);
- EXT3_INODE_GET_XTIME(i_atime, i_atime_extra, inode, raw_inode);
- EXT3_INODE_GET_XTIME(i_crtime, i_crtime_extra, ei, raw_inode);
+ EXT3_INODE_GET_XTIME(i_ctime, inode, raw_inode);
+ EXT3_INODE_GET_XTIME(i_mtime, inode, raw_inode);
+ EXT3_INODE_GET_XTIME(i_atime, inode, raw_inode);
+ EXT3_INODE_GET_XTIME(i_crtime, ei, raw_inode);
ei->i_state = 0;
ei->i_dir_start_lookup = 0;
@@ -2829,10 +2829,10 @@ static int ext3_do_update_inode(handle_t
}
raw_inode->i_links_count = cpu_to_le16(inode->i_nlink);
raw_inode->i_size = cpu_to_le32(ei->i_disksize);
- EXT3_INODE_SET_XTIME(i_ctime, i_ctime_extra, inode, raw_inode);
- EXT3_INODE_SET_XTIME(i_mtime, i_mtime_extra, inode, raw_inode);
- EXT3_INODE_SET_XTIME(i_atime, i_atime_extra, inode, raw_inode);
- EXT3_INODE_SET_XTIME(i_crtime, i_crtime_extra, ei, raw_inode);
+ EXT3_INODE_SET_XTIME(i_ctime, inode, raw_inode);
+ EXT3_INODE_SET_XTIME(i_mtime, inode, raw_inode);
+ EXT3_INODE_SET_XTIME(i_atime, inode, raw_inode);
+ EXT3_INODE_SET_XTIME(i_crtime, ei, raw_inode);
raw_inode->i_blocks = cpu_to_le32(inode->i_blocks);
raw_inode->i_dtime = cpu_to_le32(ei->i_dtime);
diff -Nurp linux.orig/include/linux/ext3_fs.h linux/include/linux/ext3_fs.h
--- linux.orig/include/linux/ext3_fs.h 2007-02-26 17:10:22.000000000 -0600
+++ linux/include/linux/ext3_fs.h 2007-02-26 17:07:20.000000000 -0600
@@ -331,37 +331,42 @@ struct ext3_inode {
#define EXT3_EPOCH_MASK ((1 << EXT3_EPOCH_BITS) - 1)
#define EXT3_NSEC_MASK (~0UL << EXT3_EPOCH_BITS)
-#define EXT3_INODE_SET_XTIME(xtime, extra_xtime, inode, raw_inode) \
-do { \
+#define EXT3_FITS_IN_INODE(ext3_inode, field) \
+ ((offsetof(typeof(*ext3_inode), field) - \
+ offsetof(typeof(*ext3_inode), i_extra_isize) + \
+ sizeof((ext3_inode)->field)) \
+ <= le16_to_cpu((ext3_inode)->i_extra_isize))
+
+static inline __le32 ext3_encode_extra_time(struct timespec *time)
+{
+ return cpu_to_le32((sizeof(time->tv_sec) > 4 ?
+ time->tv_sec >> 32 : 0) |
+ ((time->tv_nsec << 2) & EXT3_NSEC_MASK));
+}
+
+static inline void ext3_decode_extra_time(struct timespec *time, __le32 extra)
{
+ if (sizeof(time->tv_sec) > 4)
+ time->tv_sec |= (__u64)(le32_to_cpu(extra) & EXT3_EPOCH_MASK)
+ << 32;
+ time->tv_nsec = (le32_to_cpu(extra) & EXT3_NSEC_MASK) >> 2;
+}
+
+#define EXT3_INODE_SET_XTIME(xtime, inode, raw_inode) \
+do { \
(raw_inode)->xtime = cpu_to_le32((inode)->xtime.tv_sec); \
- \
- if (offsetof(typeof(*raw_inode), extra_xtime) - \
- offsetof(typeof(*raw_inode), i_extra_isize) + \
- sizeof((raw_inode)->extra_xtime) <= \
- le16_to_cpu((raw_inode)->i_extra_isize)) \
- (raw_inode)->extra_xtime = \
- cpu_to_le32((sizeof((inode)->xtime.tv_sec) > 4 ? \
- ((__u64)(inode)->xtime.tv_sec >> 32) : 0)| \
- (((inode)->xtime.tv_nsec << 2) & \
- EXT3_NSEC_MASK)); \
+ \
+ if (EXT3_FITS_IN_INODE(raw_inode, xtime ## _extra)) \
+ (raw_inode)->xtime ## _extra = \
+ ext3_encode_extra_time(&(inode)->xtime); \
} while (0)
-#define EXT3_INODE_GET_XTIME(xtime, extra_xtime, inode, raw_inode) \
-do { \
- (inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime); \
- \
- if (offsetof(typeof(*raw_inode), extra_xtime) - \
- offsetof(typeof(*raw_inode), i_extra_isize) + \
- sizeof((raw_inode)->extra_xtime) <= \
- le16_to_cpu((raw_inode)->i_extra_isize)) { \
- if (sizeof((inode)->xtime.tv_sec) > 4) \
- (inode)->xtime.tv_sec |= \
- (__u64)(le32_to_cpu((raw_inode)->extra_xtime) & \
- EXT3_EPOCH_MASK) << 32; \
- (inode)->xtime.tv_nsec = \
- (le32_to_cpu((raw_inode)->extra_xtime) & \
- EXT3_NSEC_MASK) >> 2; \
- } \
+#define EXT3_INODE_GET_XTIME(xtime, inode, raw_inode) \
+do { \
+ (inode)->xtime.tv_sec = le32_to_cpu((raw_inode)->xtime); \
+ \
+ if (EXT3_FITS_IN_INODE(raw_inode, xtime ## _extra)) \
+ ext3_decode_extra_time(&(inode)->xtime, \
+ raw_inode->xtime ## _extra); \
} while (0)
--
David Kleikamp
IBM Linux Technology Center
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
