Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1326116

Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's inode list

Show all headers | View raw

On 01/31/2016 07:04 PM, Dave Chinner wrote:
> On Fri, Jan 29, 2016 at 02:30:46PM -0500, Waiman Long wrote:
>> The inode_sb_list_add() and inode_sb_list_del() functions in the vfs
>> layer just perform list addition and deletion under lock. So they can
>> use the new list batching facility to speed up the list operations
>> when many CPUs are trying to do it simultaneously.
>>
>> In particular, the inode_sb_list_del() function can be a performance
>> bottleneck when large applications with many threads and associated
>> inodes exit. With an exit microbenchmark that creates a large number
>> of threads, attachs many inodes to them and then exits. The runtimes
>> of that microbenchmark with 1000 threads before and after the patch
> I've never seen sb inode list contention in typical workloads in
> exit processing. Can you post the test script you are using?

I have posted it in one of my earlier email.


> The inode sb list contention I usually often than not, it's
> workloads that turn over the inode cache quickly (i.e. instantiating
> lots of inodes through concurrent directory traversal or create
> workloads). These are often latency sensitive, so I'm wondering what
> the effect of spinning waiting for batch processing on every
> contended add is going to do to lookup performance...

I think the batch processor will get higher latency, but the other will 
see a shorter one. If each CPU has a more or less chance to become the 
batch processor, the overall impact to system performance should not be 
that significatn.

>> on a 4-socket Intel E7-4820 v3 system (48 cores, 96 threads) were
>> as follows:
>>
>>    Kernel        Elapsed Time    System Time
>>    ------        ------------    -----------
>>    Vanilla 4.4      65.29s         82m14s
>>    Patched 4.4      45.69s         49m44s
> I wonder if you'd get the same results on such a benchmark simply by
> making the spin lock a mutex, thereby reducing the number of CPUs
> spinning on a single lock cacheline at any one point in time.
> Certainly the system time will plummet....

I don't think it is a good idea to use mutex as we can't sleep.

>> The elapsed time and the reported system time were reduced by 30%
>> and 40% respectively.
>>
>> Signed-off-by: Waiman Long<Waiman.Long@hpe.com>
>> ---
>>   fs/inode.c         |   13 +++++--------
>>   fs/super.c         |    1 +
>>   include/linux/fs.h |    2 ++
>>   3 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/fs/inode.c b/fs/inode.c
>> index 9f62db3..870de8c 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -424,19 +424,16 @@ static void inode_lru_list_del(struct inode *inode)
>>    */
>>   void inode_sb_list_add(struct inode *inode)
>>   {
>> -	spin_lock(&inode->i_sb->s_inode_list_lock);
>> -	list_add(&inode->i_sb_list,&inode->i_sb->s_inodes);
>> -	spin_unlock(&inode->i_sb->s_inode_list_lock);
>> +	do_list_batch(&inode->i_sb->s_inode_list_lock, lb_cmd_add,
>> +			&inode->i_sb->s_list_batch,&inode->i_sb_list);
> I don't like the API. This should simply be:
>
> void inode_sb_list_add(struct inode *inode)
> {
> 	list_batch_add(&inode->i_sb_list,&inode->i_sb->s_inodes);
> }
>
> void inode_sb_list_del(struct inode *inode)
> {
> 	list_batch_del(&inode->i_sb_list,&inode->i_sb->s_inodes);
> }
>
> And all the locks, lists and batch commands are internal to the
> struct list_batch and the API implementation.
>

Points taken. I will update the patch to do that. Thanks for the review.

Cheers,
Longman

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v2 0/3] lib/list_batch: A simple list insertion/deletion batching facility Waiman Long <Waiman.Long@hpe.com> - 2016-01-29 20:40 +0100
  [PATCH v2 2/3] lib/list_batch, x86: Enable list insertion/deletion batching for x86 Waiman Long <Waiman.Long@hpe.com> - 2016-01-29 20:40 +0100
  [PATCH v2 3/3] vfs: Enable list batching for the superblock's inode list Waiman Long <Waiman.Long@hpe.com> - 2016-01-29 20:40 +0100
    Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Ingo Molnar <mingo@kernel.org> - 2016-01-30 09:40 +0100
      Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Andi Kleen <andi@firstfloor.org> - 2016-02-01 18:50 +0100
        Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Waiman Long <waiman.long@hpe.com> - 2016-02-01 23:10 +0100
          Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Waiman Long <waiman.long@hpe.com> - 2016-02-04 00:10 +0100
            Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Dave Chinner <david@fromorbit.com> - 2016-02-07 01:00 +0100
      Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Waiman Long <waiman.long@hpe.com> - 2016-02-01 22:50 +0100
    Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Dave Chinner <david@fromorbit.com> - 2016-02-01 01:10 +0100
      Re: [PATCH v2 3/3] vfs: Enable list batching for the superblock's  inode list Waiman Long <waiman.long@hpe.com> - 2016-02-04 00:10 +0100

csiph-web