From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [RFC PATCH v3 1/2] tmpfs: manage the inode-number by IDR, signed int inum Date: Tue, 3 Jun 2014 11:04:00 +0200 Message-ID: <20140603090400.GB29219@quack.suse.cz> References: <20140522151431.GA25517@lst.de> <1401639537-6449-1-git-send-email-hooanon05g@gmail.com> <1401639537-6449-2-git-send-email-hooanon05g@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org, dchinner@redhat.com, viro@zeniv.linux.org.uk, Eric Dumazet , Hugh Dickins , Christoph Hellwig , Andreas Dilger , Jan Kara To: "J. R. Okajima" Return-path: Received: from cantor2.suse.de ([195.135.220.15]:35803 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751121AbaFCJEE (ORCPT ); Tue, 3 Jun 2014 05:04:04 -0400 Content-Disposition: inline In-Reply-To: <1401639537-6449-2-git-send-email-hooanon05g@gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Mon 02-06-14 01:18:56, J. R. Okajima wrote: > To ensure the uniquness of the inode-number, manage it by IDR. > Also it tries using the lowest unused inode-number, so the value will > usually be smaller. > Another side effect is the type of the inode-number in tmpfs. By using > IDR, it is limited to signed int. But I don't think it a big > problem. INT_MAX is big enough for the number of inodes in a single tmpfs. What I'm missing with this patch is some quantification of costs this change has - i.e., it's surely going to cost us some performance. Can you measure how much? I think measuring creation and deletion of lots of empty files from 1, 2, 4, 8, .. NR_CPU processes (each process in a separate dir to avoid contention on i_mutex) in tmpfs would make sense. One correctness nit below as well. > diff --git a/mm/shmem.c b/mm/shmem.c > index 368f314..0023d8d 100644 > --- a/mm/shmem.c > +++ b/mm/shmem.c > @@ -107,9 +107,13 @@ static unsigned long shmem_default_max_blocks(void) > return totalram_pages / 2; > } > > -static unsigned long shmem_default_max_inodes(void) > +static int shmem_default_max_inodes(void) > { > - return min(totalram_pages - totalhigh_pages, totalram_pages / 2); > + int i; > + > + i = min(totalram_pages - totalhigh_pages, totalram_pages / 2); > + i = min(i, INT_MAX); With i being 'int' this doesn't make much sense... I guess it should be unsigned long. > + return i; > } > #endif Honza -- Jan Kara SUSE Labs, CR