Skip to contentSkip to conversation

kill BMAPI_UNWRITTEN

PATCH In review Christoph Hellwig 2 participants 3 messages Sep 9, 2007 – Sep 10, 2007 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kill BMAPI_UNWRITTEN

There is no reason to go through xfs_iomap for the BMAPI_UNWRITTEN because it has nothing in common with the other cases. Instead check for the shutdown filesystem in xfs_end_bio_unwritten and perform a direct call to xfs_iomap_write_unwritten (which should be renamed to something more sensible one day)

fs/xfs/linux-2.6/xfs_aops.c +3 -2
Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
@@ -237,12 +237,13 @@ xfs_end_bio_unwritten(
 {
 	xfs_ioend_t		*ioend =
 		container_of(work, xfs_ioend_t, io_work);
+	struct xfs_inode	*ip = XFS_I(ioend->io_inode);
 	xfs_off_t		offset = ioend->io_offset;
 	size_t			size = ioend->io_size;
 
 	if (likely(!ioend->io_error)) {
-		xfs_bmap(XFS_I(ioend->io_inode), offset, size,
-				BMAPI_UNWRITTEN, NULL, NULL);
+		if (!XFS_FORCED_SHUTDOWN(ip->i_mount))
+			xfs_iomap_write_unwritten(ip, offset, size);
 		xfs_setfilesize(ioend);
 	}
 	xfs_destroy_ioend(ioend);
fs/xfs/xfs_iomap.c +2 -12
Index: linux-2.6-xfs/fs/xfs/xfs_iomap.c
===================================================================
@@ -191,9 +191,7 @@ xfs_iomap(
 	if (XFS_FORCED_SHUTDOWN(mp))
 		return XFS_ERROR(EIO);
 
-	switch (flags &
-		(BMAPI_READ | BMAPI_WRITE | BMAPI_ALLOCATE |
-		 BMAPI_UNWRITTEN)) {
+	switch (flags & (BMAPI_READ | BMAPI_WRITE | BMAPI_ALLOCATE)) {
 	case BMAPI_READ:
 		xfs_iomap_enter_trace(XFS_IOMAP_READ_ENTER, io, offset, count);
 		lockmode = XFS_LCK_MAP_SHARED(mp, io);
@@ -218,8 +216,6 @@ xfs_iomap(
 			XFS_ILOCK(mp, io, lockmode);
 		}
 		break;
-	case BMAPI_UNWRITTEN:
-		goto phase2;
 	default:
 		BUG();
 	}
@@ -238,8 +234,7 @@ xfs_iomap(
 	if (error)
 		goto out;
 
-phase2:
-	switch (flags & (BMAPI_WRITE|BMAPI_ALLOCATE|BMAPI_UNWRITTEN)) {
+	switch (flags & (BMAPI_WRITE|BMAPI_ALLOCATE)) {
 	case BMAPI_WRITE:
 		/* If we found an extent, return it */
 		if (nimaps &&
@@ -277,11 +272,6 @@ phase2:
 		error = XFS_IOMAP_WRITE_ALLOCATE(mp, io, offset, count,
 						 &imap, &nimaps);
 		break;
-	case BMAPI_UNWRITTEN:
-		lockmode = 0;
-		error = XFS_IOMAP_WRITE_UNWRITTEN(mp, io, offset, count);
-		nimaps = 0;
-		break;
 	}
 
 	if (nimaps) {
fs/xfs/xfs_iomap.h +0 -1
Index: linux-2.6-xfs/fs/xfs/xfs_iomap.h
===================================================================
@@ -36,7 +36,6 @@ typedef enum {
 	BMAPI_READ = (1 << 0),		/* read extents */
 	BMAPI_WRITE = (1 << 1),		/* create extents */
 	BMAPI_ALLOCATE = (1 << 2),	/* delayed allocate to real extents */
-	BMAPI_UNWRITTEN  = (1 << 3),	/* unwritten extents to real extents */
 	/* modifiers */
 	BMAPI_IGNSTATE = (1 << 4),	/* ignore unwritten state on read */
 	BMAPI_DIRECT = (1 << 5),	/* direct instead of buffered write */
#1 lore raw

The reason for eliminating BMAPI_UNWRITTEN is not clear. It is ideal to keep all the block map
interfaces and information to go through xfs_iomap, xfs_bmapi functionality.

Thanks,
-Saradhi.

Quoted 89 lines · Christoph Hellwig

There is no reason to go through xfs_iomap for the BMAPI_UNWRITTEN because it has nothing in common with the other cases. Instead check for the shutdown filesystem in xfs_end_bio_unwritten and perform a direct call to xfs_iomap_write_unwritten (which should be renamed to something more sensible one day)

Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_aops.c 2007-09-06 10:18:
05.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_aops.c 2007-09-06 10:18:
05.000000000 +0200

fs/xfs/linux-2.6/xfs_aops.c +3 -2
@@ -237,12 +237,13 @@ xfs_end_bio_unwritten(
 {
        xfs_ioend_t             *ioend =
                container_of(work, xfs_ioend_t, io_work);
+       struct xfs_inode        *ip = XFS_I(ioend->io_inode);
        xfs_off_t               offset = ioend->io_offset;
        size_t                  size = ioend->io_size;
 
        if (likely(!ioend->io_error)) {
-               xfs_bmap(XFS_I(ioend->io_inode), offset, size,
-                               BMAPI_UNWRITTEN, NULL, NULL);
+               if (!XFS_FORCED_SHUTDOWN(ip->i_mount))
+                       xfs_iomap_write_unwritten(ip, offset, size);
                xfs_setfilesize(ioend);
        }
        xfs_destroy_ioend(ioend);

Index: linux-2.6-xfs/fs/xfs/xfs_iomap.c
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_iomap.c 2007-09-06 10:18:
05.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/xfs_iomap.c 2007-09-06 10:18:05.000000000+0200

fs/xfs/xfs_iomap.c +1 -3
@@ -191,9 +191,7 @@ xfs_iomap(
        if (XFS_FORCED_SHUTDOWN(mp))
                return XFS_ERROR(EIO);
 
-       switch (flags &
-               (BMAPI_READ | BMAPI_WRITE | BMAPI_ALLOCATE |
-                BMAPI_UNWRITTEN)) {
+       switch (flags & (BMAPI_READ | BMAPI_WRITE | BMAPI_ALLOCATE)) {
        case BMAPI_READ:
                xfs_iomap_enter_trace(XFS_IOMAP_READ_ENTER, io, offset,
 count);
                lockmode = XFS_LCK_MAP_SHARED(mp, io);
fs/xfs/xfs_iomap.c +1 -9
@@ -218,8 +216,6 @@ xfs_iomap(
                        XFS_ILOCK(mp, io, lockmode);
                }
                break;
-       case BMAPI_UNWRITTEN:
-               goto phase2;
        default:
                BUG();
        }
@@ -238,8 +234,7 @@ xfs_iomap(
        if (error)
                goto out;
 
-phase2:
-       switch (flags & (BMAPI_WRITE|BMAPI_ALLOCATE|BMAPI_UNWRITTEN)) {
+       switch (flags & (BMAPI_WRITE|BMAPI_ALLOCATE)) {
        case BMAPI_WRITE:
                /* If we found an extent, return it */
                if (nimaps &&
@@ -277,11 +272,6 @@ phase2:
                error = XFS_IOMAP_WRITE_ALLOCATE(mp, io, offset, count,
                                                 &imap, &nimaps);
                break;
-       case BMAPI_UNWRITTEN:
-               lockmode = 0;
-               error = XFS_IOMAP_WRITE_UNWRITTEN(mp, io, offset, count);
-               nimaps = 0;
-               break;
        }
 
        if (nimaps) {

Index: linux-2.6-xfs/fs/xfs/xfs_iomap.h
===================================================================
--- linux-2.6-xfs.orig/fs/xfs/xfs_iomap.h 2007-09-06 10:18:
05.000000000 +0200
+++ linux-2.6-xfs/fs/xfs/xfs_iomap.h 2007-09-06 10:18:05.000000000+0200

fs/xfs/xfs_iomap.h +0 -0
@@ -36,7 +36,6 @@ typedef enum {
        BMAPI_READ = (1 << 0),          /* read extents */
        BMAPI_WRITE = (1 << 1),         /* create extents */
        BMAPI_ALLOCATE = (1 << 2),      /* delayed allocate to real
Quoted 11 lines

extents */
- BMAPI_UNWRITTEN = (1 << 3), /* unwritten extents to real
extents */
/* modifiers */
BMAPI_IGNSTATE = (1 << 4), /* ignore unwritten state on read
*/
BMAPI_DIRECT = (1 << 5), /* direct instead of buffered
write */

[[HTML alternate version deleted]]

#2 lore raw
Quoted 3 lines · Bhagi rathi

The reason for eliminating BMAPI_UNWRITTEN is not clear. It is ideal to keep all the block map
interfaces and information to go through xfs_iomap, xfs_bmapi functionality.

Despit it's name xfs_iomap_write_unwritten is not a block map interface. Just take a look at the code, it does not use the block allocator data structures at all, and it does not do the central xfs_bmapi call (it uses xfs_bmapi later on but in very different ways).