* [PATCH 0/4] inode diet, part1
@ 2011-10-18 20:13 Christoph Hellwig
2011-10-18 20:13 ` [PATCH 1/4] xfs: make i_flags and unsigned long Christoph Hellwig
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Christoph Hellwig @ 2011-10-18 20:13 UTC (permalink / raw)
To: xfs
This is the first simple part of shrinking the in-core XFS inodes.
It replaces a completion and a waitqueue with smaller bit-keyed
synchronization, just like we do in the VFS inode.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/4] xfs: make i_flags and unsigned long 2011-10-18 20:13 [PATCH 0/4] inode diet, part1 Christoph Hellwig @ 2011-10-18 20:13 ` Christoph Hellwig 2011-10-19 0:30 ` Dave Chinner 2011-10-18 20:13 ` [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock Christoph Hellwig ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2011-10-18 20:13 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-use-i_flags --] [-- Type: text/plain, Size: 1144 bytes --] To be used for bit wakeup i_flags needs to be an unsigned long or we'll run into trouble on big endian systems. Beause of the 1-byte i_update field right after it this actually causes a fairly large size increase on it's own (4 or 7 bytes), but that increase will be more than offset by the next two patches. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-08-24 06:02:01.913971548 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-08-24 06:02:17.287221597 +0200 @@ -249,7 +249,7 @@ typedef struct xfs_inode { wait_queue_head_t i_ipin_wait; /* inode pinning wait queue */ spinlock_t i_flags_lock; /* inode i_flags lock */ /* Miscellaneous state. */ - unsigned short i_flags; /* see defined flags below */ + unsigned long i_flags; /* see defined flags below */ unsigned char i_update_core; /* timestamps/size is dirty */ unsigned int i_delayed_blks; /* count of delay alloc blks */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] xfs: make i_flags and unsigned long 2011-10-18 20:13 ` [PATCH 1/4] xfs: make i_flags and unsigned long Christoph Hellwig @ 2011-10-19 0:30 ` Dave Chinner 2011-10-19 8:57 ` Christoph Hellwig 0 siblings, 1 reply; 13+ messages in thread From: Dave Chinner @ 2011-10-19 0:30 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Tue, Oct 18, 2011 at 04:13:05PM -0400, Christoph Hellwig wrote: > To be used for bit wakeup i_flags needs to be an unsigned long or we'll > run into trouble on big endian systems. Beause of the 1-byte i_update > field right after it this actually causes a fairly large size increase > on it's own (4 or 7 bytes), but that increase will be more than offset > by the next two patches. You could always make the i_update_core boolean a flag bit, and that growth will also go away. Regardless: Reviewed-by: Dave Chinner <dchinner@redhat.com> -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/4] xfs: make i_flags and unsigned long 2011-10-19 0:30 ` Dave Chinner @ 2011-10-19 8:57 ` Christoph Hellwig 0 siblings, 0 replies; 13+ messages in thread From: Christoph Hellwig @ 2011-10-19 8:57 UTC (permalink / raw) To: Dave Chinner; +Cc: Christoph Hellwig, xfs On Wed, Oct 19, 2011 at 11:30:47AM +1100, Dave Chinner wrote: > On Tue, Oct 18, 2011 at 04:13:05PM -0400, Christoph Hellwig wrote: > > To be used for bit wakeup i_flags needs to be an unsigned long or we'll > > run into trouble on big endian systems. Beause of the 1-byte i_update > > field right after it this actually causes a fairly large size increase > > on it's own (4 or 7 bytes), but that increase will be more than offset > > by the next two patches. > > You could always make the i_update_core boolean a flag bit, and that > growth will also go away. Regardless: I could. Then again I plan to kill it for 3.3, so re-arranging the deck charis now might not be all that useful. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock 2011-10-18 20:13 [PATCH 0/4] inode diet, part1 Christoph Hellwig 2011-10-18 20:13 ` [PATCH 1/4] xfs: make i_flags and unsigned long Christoph Hellwig @ 2011-10-18 20:13 ` Christoph Hellwig 2011-10-19 0:42 ` Dave Chinner 2011-10-18 20:13 ` [PATCH 3/4] xfs: replace i_pin_wait with a bit waitqueue Christoph Hellwig 2011-10-18 20:13 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig 3 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2011-10-18 20:13 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-kill-i_flush --] [-- Type: text/plain, Size: 8458 bytes --] We almost never block on i_flock, the exception is synchronous inode flushing. Instead of bloating the inode with a 16/24-byte completion that we abuse as a semaphore just implement it as a bitlock that uses a bit waitqueue for the rare sleeping path. This primarily is a tradeoff between a much smaller inode and a faster non-blocking path vs a faster faster wakeups, and we are much better off with the former. A small downside is that we will lose lockdep checking for i_flock, but given that it's always taken inside the ilock that should be acceptable. Note that for example the inode writeback locking is implemented in a very similar way. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_iget.c =================================================================== --- xfs.orig/fs/xfs/xfs_iget.c 2011-10-18 20:55:31.093352628 +0200 +++ xfs/fs/xfs/xfs_iget.c 2011-10-18 20:58:40.141854067 +0200 @@ -77,7 +77,7 @@ xfs_inode_alloc( ASSERT(atomic_read(&ip->i_pincount) == 0); ASSERT(!spin_is_locked(&ip->i_flags_lock)); - ASSERT(completion_done(&ip->i_flush)); + ASSERT(!xfs_isiflocked(ip)); ASSERT(ip->i_ino == 0); mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino); @@ -151,7 +151,7 @@ xfs_inode_free( /* asserts to verify all state is correct here */ ASSERT(atomic_read(&ip->i_pincount) == 0); ASSERT(!spin_is_locked(&ip->i_flags_lock)); - ASSERT(completion_done(&ip->i_flush)); + ASSERT(!xfs_isiflocked(ip)); /* * Because we use RCU freeing we need to ensure the inode always @@ -716,3 +716,19 @@ xfs_isilocked( return 0; } #endif + +void +__xfs_iflock( + struct xfs_inode *ip) +{ + wait_queue_head_t *wq = bit_waitqueue(&ip->i_flags, __XFS_IFLOCK); + DEFINE_WAIT_BIT(wait, &ip->i_flags, __XFS_IFLOCK); + + do { + prepare_to_wait_exclusive(wq, &wait.wait, TASK_UNINTERRUPTIBLE); + if (xfs_isiflocked(ip)) + schedule(); + } while (!xfs_iflock_nowait(ip)); + + finish_wait(wq, &wait.wait); +} Index: xfs/fs/xfs/xfs_inode.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode.c 2011-10-18 20:55:31.069353232 +0200 +++ xfs/fs/xfs/xfs_inode.c 2011-10-18 20:58:40.141854067 +0200 @@ -2510,7 +2510,7 @@ xfs_iflush( XFS_STATS_INC(xs_iflush_count); ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)); - ASSERT(!completion_done(&ip->i_flush)); + ASSERT(xfs_isiflocked(ip)); ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE || ip->i_d.di_nextents > ip->i_df.if_ext_max); @@ -2626,7 +2626,7 @@ xfs_iflush_int( #endif ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)); - ASSERT(!completion_done(&ip->i_flush)); + ASSERT(xfs_isiflocked(ip)); ASSERT(ip->i_d.di_format != XFS_DINODE_FMT_BTREE || ip->i_d.di_nextents > ip->i_df.if_ext_max); Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-10-18 20:58:39.376854255 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-10-18 20:58:40.145858771 +0200 @@ -244,7 +244,6 @@ typedef struct xfs_inode { struct xfs_inode_log_item *i_itemp; /* logging information */ mrlock_t i_lock; /* inode lock */ mrlock_t i_iolock; /* inode IO lock */ - struct completion i_flush; /* inode flush completion q */ atomic_t i_pincount; /* inode pin count */ wait_queue_head_t i_ipin_wait; /* inode pinning wait queue */ spinlock_t i_flags_lock; /* inode i_flags lock */ @@ -331,6 +330,19 @@ xfs_iflags_test_and_clear(xfs_inode_t *i return ret; } +static inline int +xfs_iflags_test_and_set(xfs_inode_t *ip, unsigned short flags) +{ + int ret; + + spin_lock(&ip->i_flags_lock); + ret = ip->i_flags & flags; + if (!ret) + ip->i_flags |= flags; + spin_unlock(&ip->i_flags_lock); + return ret; +} + /* * Project quota id helpers (previously projid was 16bit only * and using two 16bit values to hold new 32bit projid was chosen @@ -351,26 +363,6 @@ xfs_set_projid(struct xfs_inode *ip, } /* - * Manage the i_flush queue embedded in the inode. This completion - * queue synchronizes processes attempting to flush the in-core - * inode back to disk. - */ -static inline void xfs_iflock(xfs_inode_t *ip) -{ - wait_for_completion(&ip->i_flush); -} - -static inline int xfs_iflock_nowait(xfs_inode_t *ip) -{ - return try_wait_for_completion(&ip->i_flush); -} - -static inline void xfs_ifunlock(xfs_inode_t *ip) -{ - complete(&ip->i_flush); -} - -/* * In-core inode flags. */ #define XFS_IRECLAIM 0x0001 /* started reclaiming this inode */ @@ -380,6 +372,8 @@ static inline void xfs_ifunlock(xfs_inod #define XFS_IFILESTREAM 0x0010 /* inode is in a filestream directory */ #define XFS_ITRUNCATED 0x0020 /* truncated down so flush-on-close */ #define XFS_IDIRTY_RELEASE 0x0040 /* dirty release already seen */ +#define __XFS_IFLOCK 8 /* inode is beeing flushed right now */ +#define XFS_IFLOCK (1 << __XFS_IFLOCK) /* * Per-lifetime flags need to be reset when re-using a reclaimable inode during @@ -392,6 +386,34 @@ static inline void xfs_ifunlock(xfs_inod XFS_IFILESTREAM); /* + * Synchronize processes attempting to flush the in-core inode back to disk. + */ + +extern void __xfs_iflock(struct xfs_inode *ip); + +static inline int xfs_iflock_nowait(struct xfs_inode *ip) +{ + return !xfs_iflags_test_and_set(ip, XFS_IFLOCK); +} + +static inline void xfs_iflock(struct xfs_inode *ip) +{ + if (!xfs_iflock_nowait(ip)) + __xfs_iflock(ip); +} + +static inline void xfs_ifunlock(struct xfs_inode *ip) +{ + xfs_iflags_clear(ip, XFS_IFLOCK); + wake_up_bit(&ip->i_flags, __XFS_IFLOCK); +} + +static inline int xfs_isiflocked(struct xfs_inode *ip) +{ + return xfs_iflags_test(ip, XFS_IFLOCK); +} + +/* * Flags for inode locking. * Bit ranges: 1<<1 - 1<<16-1 -- iolock/ilock modes (bitfield) * 1<<16 - 1<<32-1 -- lockdep annotation (integers) Index: xfs/fs/xfs/xfs_inode_item.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode_item.c 2011-10-18 20:55:31.085351457 +0200 +++ xfs/fs/xfs/xfs_inode_item.c 2011-10-18 20:58:40.149853334 +0200 @@ -720,7 +720,7 @@ xfs_inode_item_pushbuf( * If a flush is not in progress anymore, chances are that the * inode was taken off the AIL. So, just get out. */ - if (completion_done(&ip->i_flush) || + if (!xfs_isiflocked(ip) || !(lip->li_flags & XFS_LI_IN_AIL)) { xfs_iunlock(ip, XFS_ILOCK_SHARED); return; @@ -750,7 +750,7 @@ xfs_inode_item_push( struct xfs_inode *ip = iip->ili_inode; ASSERT(xfs_isilocked(ip, XFS_ILOCK_SHARED)); - ASSERT(!completion_done(&ip->i_flush)); + ASSERT(xfs_isiflocked(ip)); /* * Since we were able to lock the inode's flush lock and Index: xfs/fs/xfs/xfs_super.c =================================================================== --- xfs.orig/fs/xfs/xfs_super.c 2011-10-18 20:55:34.861353099 +0200 +++ xfs/fs/xfs/xfs_super.c 2011-10-18 20:58:40.149853334 +0200 @@ -838,13 +838,6 @@ xfs_fs_inode_init_once( atomic_set(&ip->i_pincount, 0); spin_lock_init(&ip->i_flags_lock); init_waitqueue_head(&ip->i_ipin_wait); - /* - * Because we want to use a counting completion, complete - * the flush completion once to allow a single access to - * the flush completion without blocking. - */ - init_completion(&ip->i_flush); - complete(&ip->i_flush); mrlock_init(&ip->i_lock, MRLOCK_ALLOW_EQUAL_PRI|MRLOCK_BARRIER, "xfsino", ip->i_ino); Index: xfs/fs/xfs/xfs_sync.c =================================================================== --- xfs.orig/fs/xfs/xfs_sync.c 2011-10-18 19:41:11.124853014 +0200 +++ xfs/fs/xfs/xfs_sync.c 2011-10-18 20:58:40.157852915 +0200 @@ -675,14 +675,13 @@ xfs_reclaim_inode_grab( return 1; /* - * do some unlocked checks first to avoid unnecessary lock traffic. - * The first is a flush lock check, the second is a already in reclaim - * check. Only do these checks if we are not going to block on locks. + * If we are beeing asked for non-blocking operation, do unlocked + * checks to see if the inode already is beeing flushed or in reclaim + * to avoid lock traffic. */ if ((flags & SYNC_TRYLOCK) && - (!ip->i_flush.done || __xfs_iflags_test(ip, XFS_IRECLAIM))) { + __xfs_iflags_test(ip, XFS_IFLOCK | XFS_IRECLAIM)) return 1; - } /* * The radix tree lock here protects a thread in xfs_iget from racing _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock 2011-10-18 20:13 ` [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock Christoph Hellwig @ 2011-10-19 0:42 ` Dave Chinner 2011-10-19 9:01 ` Christoph Hellwig 0 siblings, 1 reply; 13+ messages in thread From: Dave Chinner @ 2011-10-19 0:42 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Tue, Oct 18, 2011 at 04:13:06PM -0400, Christoph Hellwig wrote: > We almost never block on i_flock, the exception is synchronous inode > flushing. Instead of bloating the inode with a 16/24-byte completion > that we abuse as a semaphore just implement it as a bitlock that uses > a bit waitqueue for the rare sleeping path. This primarily is a > tradeoff between a much smaller inode and a faster non-blocking > path vs a faster faster wakeups, and we are much better off with > the former. > > A small downside is that we will lose lockdep checking for i_flock, but > given that it's always taken inside the ilock that should be acceptable. > > Note that for example the inode writeback locking is implemented in a > very similar way. > > Signed-off-by: Christoph Hellwig <hch@lst.de> ..... > @@ -716,3 +716,19 @@ xfs_isilocked( > return 0; > } > #endif > + > +void > +__xfs_iflock( > + struct xfs_inode *ip) > +{ > + wait_queue_head_t *wq = bit_waitqueue(&ip->i_flags, __XFS_IFLOCK); > + DEFINE_WAIT_BIT(wait, &ip->i_flags, __XFS_IFLOCK); > + > + do { > + prepare_to_wait_exclusive(wq, &wait.wait, TASK_UNINTERRUPTIBLE); > + if (xfs_isiflocked(ip)) > + schedule(); > + } while (!xfs_iflock_nowait(ip)); > + > + finish_wait(wq, &wait.wait); > +} Given that the only way that the inode will become unlocked is for IO to complete, that makes this an IO wait, right? Perhaps this should call io_schedule() in that case? > @@ -380,6 +372,8 @@ static inline void xfs_ifunlock(xfs_inod > #define XFS_IFILESTREAM 0x0010 /* inode is in a filestream directory */ > #define XFS_ITRUNCATED 0x0020 /* truncated down so flush-on-close */ > #define XFS_IDIRTY_RELEASE 0x0040 /* dirty release already seen */ > +#define __XFS_IFLOCK 8 /* inode is beeing flushed right now */ > +#define XFS_IFLOCK (1 << __XFS_IFLOCK) Any reason for leaving a gap in the flag space here? Otherwise looks good. Reviewed-by: Dave Chinner <dchinner@redhat.com> -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock 2011-10-19 0:42 ` Dave Chinner @ 2011-10-19 9:01 ` Christoph Hellwig 0 siblings, 0 replies; 13+ messages in thread From: Christoph Hellwig @ 2011-10-19 9:01 UTC (permalink / raw) To: Dave Chinner; +Cc: Christoph Hellwig, xfs On Wed, Oct 19, 2011 at 11:42:06AM +1100, Dave Chinner wrote: > > +void > > +__xfs_iflock( > > + struct xfs_inode *ip) > > +{ > > + wait_queue_head_t *wq = bit_waitqueue(&ip->i_flags, __XFS_IFLOCK); > > + DEFINE_WAIT_BIT(wait, &ip->i_flags, __XFS_IFLOCK); > > + > > + do { > > + prepare_to_wait_exclusive(wq, &wait.wait, TASK_UNINTERRUPTIBLE); > > + if (xfs_isiflocked(ip)) > > + schedule(); > > + } while (!xfs_iflock_nowait(ip)); > > + > > + finish_wait(wq, &wait.wait); > > +} > > Given that the only way that the inode will become unlocked is for > IO to complete, that makes this an IO wait, right? Perhaps this > should call io_schedule() in that case? It probably should, and would help a bit with our %iowait accounting. The biggie for that is the buffer lock, though. Either we'll need a variant of the semaphore that does io_schedule, which is probably unlikely to get given that the grater gods want struct semaphore to die. Or we'll need to do the same bitlock trick there, even if we're not too worried about struct xfs_buf size - in fact that his how fs/buffer.c gets the iowait accounting right. > > @@ -380,6 +372,8 @@ static inline void xfs_ifunlock(xfs_inod > > #define XFS_IFILESTREAM 0x0010 /* inode is in a filestream directory */ > > #define XFS_ITRUNCATED 0x0020 /* truncated down so flush-on-close */ > > #define XFS_IDIRTY_RELEASE 0x0040 /* dirty release already seen */ > > +#define __XFS_IFLOCK 8 /* inode is beeing flushed right now */ > > +#define XFS_IFLOCK (1 << __XFS_IFLOCK) > > Any reason for leaving a gap in the flag space here? I can't remember. But looking at it again it might be a good idea to convert the other flags to the (1 << bit) scheme to make the number more obvious. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] xfs: replace i_pin_wait with a bit waitqueue 2011-10-18 20:13 [PATCH 0/4] inode diet, part1 Christoph Hellwig 2011-10-18 20:13 ` [PATCH 1/4] xfs: make i_flags and unsigned long Christoph Hellwig 2011-10-18 20:13 ` [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock Christoph Hellwig @ 2011-10-18 20:13 ` Christoph Hellwig 2011-10-19 0:50 ` Dave Chinner 2011-10-18 20:13 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig 3 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2011-10-18 20:13 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-kill-i_ipin_wait --] [-- Type: text/plain, Size: 3738 bytes --] Replace i_pin_wait, which is only used during synchronous inode flushing with a bit waitqueue. This trades off a much smaller inode against slightly slower wakeup performance, and saves 12 (32-bit) or 20 (64-bit) bytes in the XFS inode. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_inode.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode.c 2011-10-18 20:58:40.141854067 +0200 +++ xfs/fs/xfs/xfs_inode.c 2011-10-18 21:01:48.296353322 +0200 @@ -2151,7 +2151,7 @@ xfs_idestroy_fork( * once someone is waiting for it to be unpinned. */ static void -xfs_iunpin_nowait( +xfs_iunpin( struct xfs_inode *ip) { ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)); @@ -2163,14 +2163,29 @@ xfs_iunpin_nowait( } +static void +__xfs_iunpin_wait( + struct xfs_inode *ip) +{ + wait_queue_head_t *wq = bit_waitqueue(&ip->i_flags, __XFS_IPINNED); + DEFINE_WAIT_BIT(q, &ip->i_flags, __XFS_IPINNED); + + xfs_iunpin(ip); + + do { + prepare_to_wait(wq, &q.wait, TASK_UNINTERRUPTIBLE); + if (xfs_ipincount(ip)) + schedule(); + } while (xfs_ipincount(ip)); + finish_wait(wq, &q.wait); +} + void xfs_iunpin_wait( struct xfs_inode *ip) { - if (xfs_ipincount(ip)) { - xfs_iunpin_nowait(ip); - wait_event(ip->i_ipin_wait, (xfs_ipincount(ip) == 0)); - } + if (xfs_ipincount(ip)) + __xfs_iunpin_wait(ip); } /* @@ -2529,7 +2544,7 @@ xfs_iflush( * out for us if they occur after the log force completes. */ if (!(flags & SYNC_WAIT) && xfs_ipincount(ip)) { - xfs_iunpin_nowait(ip); + xfs_iunpin(ip); xfs_ifunlock(ip); return EAGAIN; } Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-10-18 20:58:40.145858771 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-10-18 21:15:23.224854073 +0200 @@ -245,7 +245,6 @@ typedef struct xfs_inode { mrlock_t i_lock; /* inode lock */ mrlock_t i_iolock; /* inode IO lock */ atomic_t i_pincount; /* inode pin count */ - wait_queue_head_t i_ipin_wait; /* inode pinning wait queue */ spinlock_t i_flags_lock; /* inode i_flags lock */ /* Miscellaneous state. */ unsigned long i_flags; /* see defined flags below */ @@ -374,6 +373,7 @@ xfs_set_projid(struct xfs_inode *ip, #define XFS_IDIRTY_RELEASE 0x0040 /* dirty release already seen */ #define __XFS_IFLOCK 8 /* inode is beeing flushed right now */ #define XFS_IFLOCK (1 << __XFS_IFLOCK) +#define __XFS_IPINNED 9 /* wakeup key for zero pin count */ /* * Per-lifetime flags need to be reset when re-using a reclaimable inode during Index: xfs/fs/xfs/xfs_inode_item.c =================================================================== --- xfs.orig/fs/xfs/xfs_inode_item.c 2011-10-18 20:58:40.149853334 +0200 +++ xfs/fs/xfs/xfs_inode_item.c 2011-10-18 21:01:48.300352725 +0200 @@ -559,7 +559,7 @@ xfs_inode_item_unpin( trace_xfs_inode_unpin(ip, _RET_IP_); ASSERT(atomic_read(&ip->i_pincount) > 0); if (atomic_dec_and_test(&ip->i_pincount)) - wake_up(&ip->i_ipin_wait); + wake_up_bit(&ip->i_flags, __XFS_IPINNED); } /* Index: xfs/fs/xfs/xfs_super.c =================================================================== --- xfs.orig/fs/xfs/xfs_super.c 2011-10-18 20:58:40.149853334 +0200 +++ xfs/fs/xfs/xfs_super.c 2011-10-18 21:01:48.304357570 +0200 @@ -837,7 +837,6 @@ xfs_fs_inode_init_once( /* xfs inode */ atomic_set(&ip->i_pincount, 0); spin_lock_init(&ip->i_flags_lock); - init_waitqueue_head(&ip->i_ipin_wait); mrlock_init(&ip->i_lock, MRLOCK_ALLOW_EQUAL_PRI|MRLOCK_BARRIER, "xfsino", ip->i_ino); _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/4] xfs: replace i_pin_wait with a bit waitqueue 2011-10-18 20:13 ` [PATCH 3/4] xfs: replace i_pin_wait with a bit waitqueue Christoph Hellwig @ 2011-10-19 0:50 ` Dave Chinner 0 siblings, 0 replies; 13+ messages in thread From: Dave Chinner @ 2011-10-19 0:50 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Tue, Oct 18, 2011 at 04:13:07PM -0400, Christoph Hellwig wrote: > Replace i_pin_wait, which is only used during synchronous inode flushing > with a bit waitqueue. This trades off a much smaller inode against > slightly slower wakeup performance, and saves 12 (32-bit) or 20 (64-bit) > bytes in the XFS inode. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > > Index: xfs/fs/xfs/xfs_inode.c > =================================================================== > --- xfs.orig/fs/xfs/xfs_inode.c 2011-10-18 20:58:40.141854067 +0200 > +++ xfs/fs/xfs/xfs_inode.c 2011-10-18 21:01:48.296353322 +0200 > @@ -2151,7 +2151,7 @@ xfs_idestroy_fork( > * once someone is waiting for it to be unpinned. > */ > static void > -xfs_iunpin_nowait( > +xfs_iunpin( > struct xfs_inode *ip) > { > ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)); > @@ -2163,14 +2163,29 @@ xfs_iunpin_nowait( > > } > > +static void > +__xfs_iunpin_wait( > + struct xfs_inode *ip) > +{ > + wait_queue_head_t *wq = bit_waitqueue(&ip->i_flags, __XFS_IPINNED); > + DEFINE_WAIT_BIT(q, &ip->i_flags, __XFS_IPINNED); > + > + xfs_iunpin(ip); > + > + do { > + prepare_to_wait(wq, &q.wait, TASK_UNINTERRUPTIBLE); > + if (xfs_ipincount(ip)) > + schedule(); > + } while (xfs_ipincount(ip)); > + finish_wait(wq, &q.wait); > +} Same comment about io_schedule() here - it's an IO we're waiting to complete here. And it's not an exclusive wait because we can have multiple callers waiting on the inode being unpinned and we want them all woken in one go? > @@ -374,6 +373,7 @@ xfs_set_projid(struct xfs_inode *ip, > #define XFS_IDIRTY_RELEASE 0x0040 /* dirty release already seen */ > #define __XFS_IFLOCK 8 /* inode is beeing flushed right now */ > #define XFS_IFLOCK (1 << __XFS_IFLOCK) > +#define __XFS_IPINNED 9 /* wakeup key for zero pin count */ Should you also define XFS_IPINNED for consistency, even though it is not used? Otherwise looks OK. Cheers, Dave. -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] xfs: remove the unused dm_attrs structure 2011-10-18 20:13 [PATCH 0/4] inode diet, part1 Christoph Hellwig ` (2 preceding siblings ...) 2011-10-18 20:13 ` [PATCH 3/4] xfs: replace i_pin_wait with a bit waitqueue Christoph Hellwig @ 2011-10-18 20:13 ` Christoph Hellwig 2011-10-19 0:50 ` Dave Chinner 3 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2011-10-18 20:13 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-remove-dm_attrs --] [-- Type: text/plain, Size: 1029 bytes --] .. and the just as dead bhv_desc forward declaration while we're at it. Signed-off-by: Christoph Hellwig <hch@lst.de> Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-10-18 21:32:14.225353097 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-10-18 21:32:23.404480427 +0200 @@ -211,7 +211,6 @@ typedef struct xfs_icdinode { #ifdef __KERNEL__ -struct bhv_desc; struct xfs_buf; struct xfs_bmap_free; struct xfs_bmbt_irec; @@ -220,12 +219,6 @@ struct xfs_mount; struct xfs_trans; struct xfs_dquot; -typedef struct dm_attrs_s { - __uint32_t da_dmevmask; /* DMIG event mask */ - __uint16_t da_dmstate; /* DMIG state info */ - __uint16_t da_pad; /* DMIG extra padding */ -} dm_attrs_t; - typedef struct xfs_inode { /* Inode linking and identification information. */ struct xfs_mount *i_mount; /* fs mount struct ptr */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] xfs: remove the unused dm_attrs structure 2011-10-18 20:13 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig @ 2011-10-19 0:50 ` Dave Chinner 0 siblings, 0 replies; 13+ messages in thread From: Dave Chinner @ 2011-10-19 0:50 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Tue, Oct 18, 2011 at 04:13:08PM -0400, Christoph Hellwig wrote: > .. and the just as dead bhv_desc forward declaration while we're at it. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> -- Dave Chinner david@fromorbit.com _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 0/4] inode diet, part1 V2 @ 2011-10-19 18:23 Christoph Hellwig 2011-10-19 18:23 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig 0 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2011-10-19 18:23 UTC (permalink / raw) To: xfs This is the first simple part of shrinking the in-core XFS inodes. It replaces a completion and a waitqueue with smaller bit-keyed synchronization, just like we do in the VFS inode. _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] xfs: remove the unused dm_attrs structure 2011-10-19 18:23 [PATCH 0/4] inode diet, part1 V2 Christoph Hellwig @ 2011-10-19 18:23 ` Christoph Hellwig 2011-10-26 21:07 ` Alex Elder 0 siblings, 1 reply; 13+ messages in thread From: Christoph Hellwig @ 2011-10-19 18:23 UTC (permalink / raw) To: xfs [-- Attachment #1: xfs-remove-dm_attrs --] [-- Type: text/plain, Size: 1077 bytes --] .. and the just as dead bhv_desc forward declaration while we're at it. Signed-off-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Dave Chinner <dchinner@redhat.com> Index: xfs/fs/xfs/xfs_inode.h =================================================================== --- xfs.orig/fs/xfs/xfs_inode.h 2011-10-18 21:32:14.225353097 +0200 +++ xfs/fs/xfs/xfs_inode.h 2011-10-18 21:32:23.404480427 +0200 @@ -211,7 +211,6 @@ typedef struct xfs_icdinode { #ifdef __KERNEL__ -struct bhv_desc; struct xfs_buf; struct xfs_bmap_free; struct xfs_bmbt_irec; @@ -220,12 +219,6 @@ struct xfs_mount; struct xfs_trans; struct xfs_dquot; -typedef struct dm_attrs_s { - __uint32_t da_dmevmask; /* DMIG event mask */ - __uint16_t da_dmstate; /* DMIG state info */ - __uint16_t da_pad; /* DMIG extra padding */ -} dm_attrs_t; - typedef struct xfs_inode { /* Inode linking and identification information. */ struct xfs_mount *i_mount; /* fs mount struct ptr */ _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] xfs: remove the unused dm_attrs structure 2011-10-19 18:23 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig @ 2011-10-26 21:07 ` Alex Elder 0 siblings, 0 replies; 13+ messages in thread From: Alex Elder @ 2011-10-26 21:07 UTC (permalink / raw) To: Christoph Hellwig; +Cc: xfs On Wed, 2011-10-19 at 14:23 -0400, Christoph Hellwig wrote: > .. and the just as dead bhv_desc forward declaration while we're at it. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Dave Chinner <dchinner@redhat.com> Looks good. Reviewed-by: Alex Elder <aelder@sgi.com> _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2011-10-26 21:07 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-10-18 20:13 [PATCH 0/4] inode diet, part1 Christoph Hellwig 2011-10-18 20:13 ` [PATCH 1/4] xfs: make i_flags and unsigned long Christoph Hellwig 2011-10-19 0:30 ` Dave Chinner 2011-10-19 8:57 ` Christoph Hellwig 2011-10-18 20:13 ` [PATCH 2/4] xfs: replace i_flock with a sleeping bitlock Christoph Hellwig 2011-10-19 0:42 ` Dave Chinner 2011-10-19 9:01 ` Christoph Hellwig 2011-10-18 20:13 ` [PATCH 3/4] xfs: replace i_pin_wait with a bit waitqueue Christoph Hellwig 2011-10-19 0:50 ` Dave Chinner 2011-10-18 20:13 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig 2011-10-19 0:50 ` Dave Chinner -- strict thread matches above, loose matches on Subject: below -- 2011-10-19 18:23 [PATCH 0/4] inode diet, part1 V2 Christoph Hellwig 2011-10-19 18:23 ` [PATCH 4/4] xfs: remove the unused dm_attrs structure Christoph Hellwig 2011-10-26 21:07 ` Alex Elder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox