Re: [PATCH v4 3/6] xfs: implement write-stream management support

[Christoph Hellwig <hch@lst.de>]

> +int
> +xfs_inode_max_write_streams(
> +	struct xfs_inode	*ip)
> +{
> +	struct block_device	*bdev;
> +	bool			is_filestream, is_realtime;
> +
> +	xfs_ilock(ip, XFS_ILOCK_SHARED);
> +	is_filestream = xfs_inode_is_filestream(ip);
> +	is_realtime = XFS_IS_REALTIME_INODE(ip);
> +	bdev = xfs_inode_buftarg(ip)->bt_bdev;
> +	xfs_iunlock(ip, XFS_ILOCK_SHARED);
> +
> +	if (!bdev || is_filestream || is_realtime)
> +		return 0;

All this information is stale as soon as soon as the lock is
dropped.  Also a NULL bdev can only happen for the in-memory
buftarg used by repair, so no need to check this.

> +uint16_t
> +xfs_inode_get_write_stream(
> +	struct xfs_inode	*ip)
> +{
> +	uint16_t	stream_id;
> +
> +	xfs_ilock(ip, XFS_ILOCK_SHARED);
> +	stream_id = ip->i_write_stream;
> +	xfs_iunlock(ip, XFS_ILOCK_SHARED);

Same here.  READ_ONCE/WRITE_ONCE might be a better idea to simply
avoid the lock at read time.

> +{
> +	struct xfs_write_stream	*ws = file->private_data;
> +	struct xfs_mount	*mp = ws->mp;
> +
> +	spin_lock(&mp->m_streams_lock);
> +	clear_bit(ws->stream_id - 1, mp->m_streams_in_use);

Given that all accesses to the m_streams_in_use bitmap use a lock,
there is no need for the atomic bitops.  We could use the __-versions
or just code the logic.  Or not bother because none of this is
performance critical :)

> +	ws = fd_file(f)->private_data;
> +	if (ws->mp != ip->i_mount)
> +		return -EINVAL;
> +
> +	xfs_ilock(ip, XFS_ILOCK_EXCL);
> +
> +	if (XFS_IS_REALTIME_INODE(ip) || xfs_inode_is_filestream(ip) ||
> +	    VFS_I(ip)->i_write_hint != WRITE_LIFE_NOT_SET) {

If we want to enforce a hint vs stream exlusion we'd also need to
do this when setting the hints.  Another approach might be to allow
them to coexist, and set a lifetime hint on each stream, which the
inode must match.  Although I'm not sure this would be all that
useful.