From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liu Aleaxander Subject: Re: [PATCH] vfs: does call expand_files when needed Date: Wed, 18 Nov 2009 16:54:04 +0800 Message-ID: References: <2375c9f90911172317x781e22a9y56ecb8e682e8e061@mail.gmail.com> <2375c9f90911180035v45b3f732gfe898092583a667a@mail.gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Alexander Viro , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org To: =?ISO-8859-1?Q?Am=E9rico_Wang?= Return-path: Received: from mail-px0-f180.google.com ([209.85.216.180]:38288 "EHLO mail-px0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752485AbZKRIx6 convert rfc822-to-8bit (ORCPT ); Wed, 18 Nov 2009 03:53:58 -0500 In-Reply-To: <2375c9f90911180035v45b3f732gfe898092583a667a@mail.gmail.com> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Wed, Nov 18, 2009 at 4:35 PM, Am=E9rico Wang wrote: > On Wed, Nov 18, 2009 at 3:41 PM, Liu Aleaxander wrote: >> On Wed, Nov 18, 2009 at 3:17 PM, Am=E9rico Wang wrote: >>> >>> On Wed, Nov 18, 2009 at 1:54 PM, Liu Aleaxander wrote: >>> > From: Liu Aleaxander >>> > Date: Wed, 18 Nov 2009 10:59:09 +0800 >>> > Subject: [PATCH] vfs: does call expand_files when needed >>> > >>> > I don't think we should call expand_files every time we open a >>> > file for a new unused fd, so does the expand when necessary. >>> > >>> > Signed-off-by: Liu Aleaxander >>> > --- >>> > =A0fs/file.c | =A0 27 ++++++++++++++------------- >>> > =A01 files changed, 14 insertions(+), 13 deletions(-) >>> > >>> > diff --git a/fs/file.c b/fs/file.c >>> > index 87e1290..3f3d0fc 100644 >>> > --- a/fs/file.c >>> > +++ b/fs/file.c >>> > @@ -452,22 +452,22 @@ repeat: >>> > =A0 =A0 =A0 =A0if (fd < files->next_fd) >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fd =3D files->next_fd; >>> > >>> > - =A0 =A0 =A0 if (fd < fdt->max_fds) >>> > + =A0 =A0 =A0 if (likely(fd < fdt->max_fds)) { >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fd =3D find_next_zero_bit(fdt->ope= n_fds->fds_bits, >>> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= =A0 =A0 =A0 =A0 fdt->max_fds, fd); >>> > - >>> > - =A0 =A0 =A0 error =3D expand_files(files, fd); >>> > - =A0 =A0 =A0 if (error < 0) >>> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto out; >>> > - >>> > - =A0 =A0 =A0 /* >>> > - =A0 =A0 =A0 =A0* If we needed to expand the fs array we >>> > - =A0 =A0 =A0 =A0* might have blocked - try again. >>> > - =A0 =A0 =A0 =A0*/ >>> > - =A0 =A0 =A0 if (error) >>> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto repeat; >>> > - >>> > + =A0 =A0 =A0 } else { >>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 error =3D expand_files(files, fd); >>> >>> >>> In expand_files(), it has the check for >>> ' < fdt->max_fds', so this change is not necessary. >> >> Yeah, indeed. But why we should go into an another function to do a >> _double_ check especially we mostly don't need to do that? > > You only optimized one call path, Yes, and that's the intent of this patch. >it's trivial, not so much an improvement, IMO. So, shouldn't we do the optimize when there is a way to do that? While, I don't think so. And BTW, it's not just a problem of optimization, but also make it be more sense: JUST call expand when need. I don't know why you are rejecting about this, especially it did optimized one call path(as you said), and it doesn't make the code uglier than before but making it be more sense, and, in fact, a kind of more readable. --=20 regards Liu Aleaxander -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html