Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Andreas Dilger <adilger@dilger.ca>
Message-Id: <D81FA0ED-7F7D-4B57-BE9A-26C5941D8FFE@dilger.ca>
Content-Type: multipart/signed;
 boundary="Apple-Mail=_E49322AE-A695-47B8-8F15-BAD54F0C2D97";
 protocol="application/pgp-signature"; micalg=pgp-sha256
Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\))
Subject: Re: [RFC][PATCHSET] sorting out RCU-delayed stuff in
 ->destroy_inode()
Date:   Mon, 29 Apr 2019 23:26:15 -0600
In-Reply-To: <20190430042623.GJ23075@ZenIV.linux.org.uk>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Linux List Kernel Mailing <linux-kernel@vger.kernel.org>
To:     Al Viro <viro@zeniv.linux.org.uk>
References: <20190416174900.GT2217@ZenIV.linux.org.uk>
 <CAHk-=wh6cSEztastk6-A0HUSLtJT=9W38xMN5ht-OOAnL80jxg@mail.gmail.com>
 <20190430030914.GF23075@ZenIV.linux.org.uk>
 <F01D238D-8A6C-4629-ABC5-4A8BAC25951F@dilger.ca>
 <20190430042623.GJ23075@ZenIV.linux.org.uk>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--Apple-Mail=_E49322AE-A695-47B8-8F15-BAD54F0C2D97
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii


> On Apr 29, 2019, at 10:26 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>=20
> On Mon, Apr 29, 2019 at 10:18:04PM -0600, Andreas Dilger wrote:
>>>=20
>>> 	void			*i_private; /* fs or device private =
pointer */
>>> +	void (*free_inode)(struct inode *);
>>=20
>> It seems like a waste to increase the size of every struct inode just =
to access
>> a static pointer.  Is this the only place that ->free_inode() is =
called?  Why
>> not move the ->free_inode() pointer into inode->i_fop->free_inode() =
so that it
>> is still directly accessible at this point.
>=20
> i_op, surely?

Yes, i_op is what I was thinking.

> In any case, increasing sizeof(struct inode) is not a problem -

> if anything, I'd turn ->i_fop into an anon union with that.  As in,
>=20
> diff --git a/fs/inode.c b/fs/inode.c
> index fb45590d284e..627e1766503a 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -211,8 +211,8 @@ EXPORT_SYMBOL(free_inode_nonrcu);
> static void i_callback(struct rcu_head *head)
> {
> 	struct inode *inode =3D container_of(head, struct inode, i_rcu);
> -	if (inode->i_sb->s_op->free_inode)
> -		inode->i_sb->s_op->free_inode(inode);
> +	if (inode->free_inode)
> +		inode->free_inode(inode);
> 	else
> 		free_inode_nonrcu(inode);
> }
> @@ -236,6 +236,7 @@ static struct inode *alloc_inode(struct =
super_block *sb)
> 			if (!ops->free_inode)
> 				return NULL;
> 		}
> +		inode->free_inode =3D ops->free_inode;
> 		i_callback(&inode->i_rcu);
> 		return NULL;
> 	}

> @@ -276,6 +277,7 @@ static void destroy_inode(struct inode *inode)
> 		if (!ops->free_inode)
> 			return;
> 	}
> +	inode->free_inode =3D ops->free_inode;
> 	call_rcu(&inode->i_rcu, i_callback);
> }

This seems like kind of a hack.  I guess your goal is to have =
->free_inode
accessible regardless of whether the filesystem has installed its own =
->i_op
methods or not, and i_fop is no longer used by this point.

That said, this seems better than increasing the size of struct inode.

> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 2e9b9f87caca..92732286b748 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -694,7 +694,10 @@ struct inode {
> #ifdef CONFIG_IMA
> 	atomic_t		i_readcount; /* struct files open RO */
> #endif
> -	const struct file_operations	*i_fop;	/* former =
->i_op->default_file_ops */
> +	union {
> +		const struct file_operations	*i_fop;	/* former =
->i_op->default_file_ops */
> +		void (*free_inode)(struct inode *);
> +	};


Cheers, Andreas


--Apple-Mail=_E49322AE-A695-47B8-8F15-BAD54F0C2D97
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
	filename=signature.asc
Content-Type: application/pgp-signature;
	name=signature.asc
Content-Description: Message signed with OpenPGP

-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org

iQIzBAEBCAAdFiEEDb73u6ZejP5ZMprvcqXauRfMH+AFAlzH3HgACgkQcqXauRfM
H+B8Uw/+Pe3e89BpYfhV3O4jATQofqb9PH/+i8TAZmKTVOC09IO5ole1zpaWHGV+
8FQmrVvHUrUZLvz3GZJYAA7djWqJHIN58yPUllps35f6SGqfj54rEwwq3QAFSqFR
kOiDHHtWpASu0oVRDG1Lw0xijx8wrTmfmdWOTTYSL0o58uFCFhcuBdU9oaBi/Wrs
9RHwrQb4pffl6aCMP6FTVp+DdqVtIRMQ4UNOKrJiwa8SLuJSCw1Nwj7tzwOIXu7e
z2R9sFUQHOVx1r7cOgCrP+zpIZTNmOI7lsJnPUwgod5i4lLJqBylrGmz9B/lWbln
2PPtaUkFbB1VVnhxnz4K06Fp2AEE3S8DEzAwEyitCnb+FKvV9fCdRC/m/mx1pFhj
Icc/L+2F8xuYcw1EqB/Nyc2UAVs7VgpUEnedUUwBWjg+Md5zT+BeZY6qOxzwGhXq
vp3O2hSlRhNjgsGVpkWt7awa6JlUbvw1F76ktl2tiqiWl45X75FOS2RyjLxFt8Yw
FIqp29O8HauMdaEQb1CXvgemvnzM3f6om8NCN6q+zAz8RUjwK6N1x+z0f95iHEq9
ZXxY4KYHW3cjhlhKyZpiowqlMq+1I43NWhJFhMi2AVrEzDBBleBSdH617/R6hNbd
XJMyvjDo67gvOJ1UJeX+wAISpQz5M6l6EEZqoe/GcZTEtR9UNfw=
=a4F4
-----END PGP SIGNATURE-----

--Apple-Mail=_E49322AE-A695-47B8-8F15-BAD54F0C2D97--