On 02/14/2017 01:43 AM, Christoph Hellwig wrote:

On Mon, Feb 13, 2017 at 08:46:02PM -0600, Goldwyn Rodrigues wrote:

quoted

From: Goldwyn Rodrigues <redacted>

Please Cc the while series to all lists, otherwiwe it's impossible
to review the thing.

quoted

If IOCB_NONBLOCKING is set:
	+ Check if writing beyond end of file, if yes return EAGAIN
	- check if writing to a hole which does not have allocated
	  file blocks.
	- Check if i_rwsem is immediately lockable in xfs_rw_ilock()

Why the + vs - above? 

A mistake. It does not mean anything besides bullets.

quoted

-static inline void
+static inline int
 xfs_rw_ilock(

This function has been removed a while ago.

And thanks for putting in xfs_ilock_nowait(). This can't be done in a
cleaner way. I was very skeptical of adding yet another flag.

quoted

 
+	if (iocb->ki_flags & IOCB_NONBLOCKING) {
+		struct xfs_bmbt_irec	imap;
+		xfs_fileoff_t           offset_fsb, end_fsb;
+		int			nimaps = 1, ret = 0;
+		end_fsb = XFS_B_TO_FSB(ip->i_mount, iocb->ki_pos + count);
+		if (XFS_B_TO_FSB(ip->i_mount, i_size_read(inode)) < end_fsb)
+			return -EAGAIN;

Bogus check, XFS supports async write beyond EOF if the space is
preallocated.

I assume this will be covered by xfs_bmapi_write().

quoted

+		/* Check if it is an unallocated hole */
+		offset_fsb = XFS_B_TO_FSBT(ip->i_mount, iocb->ki_pos);
+
+		ret = xfs_bmapi_read(ip, offset_fsb, end_fsb - offset_fsb, &imap,
+				&nimaps, 0);
+		if (ret)
+			return ret;
+		if (!nimaps || imap.br_startblock == HOLESTARTBLOCK)
+			return -EAGAIN;

This would need the ilock.  But extent list lookups are expensive,
so please don't add another one here.  Just return when we hit the
first unallocated extent in xfs_bmapi_write - either a short write or
-EAGAIN if nothing has been written.

So in xfs_bmapi_write (and referring to), I would need a new flag, say
XFS_BMAPI_NOALLOC which would bail to error0, setting error=-EAGAIN if
need_alloc or was_delay is set. This flag XFS_BMAPI_NOALLOC is set in
xfs_iomap_write_direct(). I did not understand short writes. Where can I
get a short write?

If I understand correctly, we do add the flag.

quoted

 	error = xfs_break_layouts(inode, iolock, true);
 	if (error)
 		return error;

Additionally this can drop the iolock, so you might get a new
hole after it.

-- 
Goldwyn