From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [PATCH] cifs: implement drop_inode superblock op Date: Tue, 25 May 2010 20:09:20 -0400 Message-ID: <20100525200920.774cf77e@corrin.poochiereds.net> References: <1274815488-29173-1-git-send-email-jlayton@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-cifs-client@lists.samba.org, linux-fsdevel@vger.kernel.org To: Steve French Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57027 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758535Ab0EZAHy convert rfc822-to-8bit (ORCPT ); Tue, 25 May 2010 20:07:54 -0400 In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, 25 May 2010 17:14:10 -0500 Steve French wrote: > Any rough idea of performance or memory savings (even in something > artificial like dbench run)? >=20 It's more of a memory savings thing. When I mount with -o noserverino and run fsstress on the mount, I'd regularly see the size of the cifs_inode_cache hit 60M or more (on a client with 1G RAM). With this patch in place, it rarely goes over 2M in size. Eventually, memory pressure will force the size to go down, but if we know that they'll never be used again (which is the case with noserverino), it's better to go ahead and just free them. > On Tue, May 25, 2010 at 2:24 PM, Jeff Layton wro= te: > > The standard behavior for drop_inode is to delete the inode when th= e > > last reference to it is put and the nlink count goes to 0. This hel= ps > > keep inodes that are still considered "not deleted" in cache as lon= g as > > possible even when there aren't dentries attached to them. > > > > When server inode numbers are disabled, it's not possible for cifs_= iget > > to ever match an existing inode (since inode numbers are generated = via > > iunique). In this situation, cifs can keep a lot of inodes in cache= that > > will never be used again. > > > > Implement a drop_inode routine that deletes the inode if server ino= de > > numbers are disabled on the mount. This helps keep the cifs inode > > caches down to a more manageable size when server inode numbers are > > disabled. > > > > Signed-off-by: Jeff Layton > > --- > > =A0fs/cifs/cifsfs.c | =A0 14 ++++++++++++-- > > =A01 files changed, 12 insertions(+), 2 deletions(-) > > > > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c > > index 78c02eb..8f647db 100644 > > --- a/fs/cifs/cifsfs.c > > +++ b/fs/cifs/cifsfs.c > > @@ -473,13 +473,23 @@ static int cifs_remount(struct super_block *s= b, int *flags, char *data) > > =A0 =A0 =A0 =A0return 0; > > =A0} > > > > +void cifs_drop_inode(struct inode *inode) > > +{ > > + =A0 =A0 =A0 struct cifs_sb_info *cifs_sb =3D CIFS_SB(inode->i_sb)= ; > > + > > + =A0 =A0 =A0 if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM) > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return generic_drop_inode(inode); > > + > > + =A0 =A0 =A0 return generic_delete_inode(inode); > > +} > > + > > =A0static const struct super_operations cifs_super_ops =3D { > > =A0 =A0 =A0 =A0.put_super =3D cifs_put_super, > > =A0 =A0 =A0 =A0.statfs =3D cifs_statfs, > > =A0 =A0 =A0 =A0.alloc_inode =3D cifs_alloc_inode, > > =A0 =A0 =A0 =A0.destroy_inode =3D cifs_destroy_inode, > > -/* =A0 =A0 .drop_inode =A0 =A0 =A0 =A0 =3D generic_delete_inode, > > - =A0 =A0 =A0 .delete_inode =A0 =3D cifs_delete_inode, =A0*/ =A0/* = Do not need above two > > + =A0 =A0 =A0 .drop_inode =A0 =A0 =3D cifs_drop_inode, > > +/* =A0 =A0 .delete_inode =A0 =3D cifs_delete_inode, =A0*/ =A0/* Do= not need above two > > =A0 =A0 =A0 =A0functions unless later we add lazy close of inodes o= r unless the > > =A0 =A0 =A0 =A0kernel forgets to call us with the same number of re= leases (closes) > > =A0 =A0 =A0 =A0as opens */ > > -- > > 1.6.6.1 > > > > >=20 >=20 >=20 --=20 Jeff Layton -- 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