[PATCH] fuse: avoid scheduling while atomic

[PATCH] fuse: avoid scheduling while atomic

[Maxim Patlasov]

As reported by Richard Sharpe, an attempt to use fuse_notify_inval_entry()
triggers complains about scheduling while atomic:

> Jun 23 11:53:24 localhost kernel: BUG: scheduling while atomic:
fuse.hf/13976/0x10000001

This happens because fuse_notify_inval_entry() attempts to allocate memory
with GFP_KERNEL, holding "struct fuse_copy_state" mapped by kmap_atomic().

The patch fixes the problem for fuse_notify_inval_entry() and other notifiers
by unmapping fuse_copy_state before allocations and remapping it again
afterwards.

Reported-by: Richard Sharpe <realrichardsharpe@gmail.com>
Signed-off-by: Maxim Patlasov <mpatlasov@parallels.com>
---
 fs/fuse/dev.c |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 098f97b..502aa1d 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -755,6 +755,22 @@ static int fuse_copy_fill(struct fuse_copy_state *cs)
 	return lock_request(cs->fc, cs->req);
 }
 
+static void fuse_copy_unmap(struct fuse_copy_state *cs)
+{
+	cs->buf = (void *)(cs->buf - cs->mapaddr);
+	kunmap_atomic(cs->mapaddr);
+}
+
+static void fuse_copy_remap(struct fuse_copy_state *cs)
+{
+	if (cs->currbuf)
+		cs->mapaddr = kmap_atomic(cs->currbuf->page);
+	else
+		cs->mapaddr = kmap_atomic(cs->pg);
+
+	cs->buf = (unsigned long)cs->buf + cs->mapaddr;
+}
+
 /* Do as much copy to/from userspace buffer as we can */
 static int fuse_copy_do(struct fuse_copy_state *cs, void **val, unsigned *size)
 {
@@ -1431,7 +1447,9 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
 	char *buf;
 	struct qstr name;
 
+	fuse_copy_unmap(cs);
 	buf = kzalloc(FUSE_NAME_MAX + 1, GFP_KERNEL);
+	fuse_copy_remap(cs);
 	if (!buf)
 		goto err;
 
@@ -1482,7 +1500,9 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
 	char *buf;
 	struct qstr name;
 
+	fuse_copy_unmap(cs);
 	buf = kzalloc(FUSE_NAME_MAX + 1, GFP_KERNEL);
+	fuse_copy_remap(cs);
 	if (!buf)
 		goto err;
 
@@ -1554,6 +1574,7 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
 
 	nodeid = outarg.nodeid;
 
+	fuse_copy_unmap(cs);
 	down_read(&fc->killsb);
 
 	err = -ENOENT;
@@ -1586,7 +1607,9 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
 			goto out_iput;
 
 		this_num = min_t(unsigned, num, PAGE_CACHE_SIZE - offset);
+		fuse_copy_remap(cs);
 		err = fuse_copy_page(cs, &page, offset, this_num, 0);
+		fuse_copy_unmap(cs);
 		if (!err && offset == 0 &&
 		    (this_num == PAGE_CACHE_SIZE || file_size == end))
 			SetPageUptodate(page);
@@ -1607,6 +1630,7 @@ out_iput:
 	iput(inode);
 out_up_killsb:
 	up_read(&fc->killsb);
+	fuse_copy_remap(cs);
 out_finish:
 	fuse_copy_finish(cs);
 	return err;

Re: [PATCH] fuse: avoid scheduling while atomic

[Miklos Szeredi]

Maxim, thanks for the patch.

Here's a reworked one.  While it looks a bit more complicated, its chances of
being (and remaining) correct are, I think, better, since the region surrounded
by kmap/kunmap_atomic is trivially non-sleeping.  This patch also removes more
lines than it adds, as an added bonus.

Please review and test if possible.

Thanks,
Miklos

---
 fs/fuse/dev.c |   51 +++++++++++++++++++++++----------------------------
 1 file changed, 23 insertions(+), 28 deletions(-)

--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -643,9 +643,8 @@ struct fuse_copy_state {
 	unsigned long seglen;
 	unsigned long addr;
 	struct page *pg;
-	void *mapaddr;
-	void *buf;
 	unsigned len;
+	unsigned offset;
 	unsigned move_pages:1;
 };
 
@@ -666,23 +665,17 @@ static void fuse_copy_finish(struct fuse
 	if (cs->currbuf) {
 		struct pipe_buffer *buf = cs->currbuf;
 
-		if (!cs->write) {
-			kunmap_atomic(cs->mapaddr);
-		} else {
-			kunmap_atomic(cs->mapaddr);
+		if (cs->write)
 			buf->len = PAGE_SIZE - cs->len;
-		}
 		cs->currbuf = NULL;
-		cs->mapaddr = NULL;
-	} else if (cs->mapaddr) {
-		kunmap_atomic(cs->mapaddr);
+	} else if (cs->pg) {
 		if (cs->write) {
 			flush_dcache_page(cs->pg);
 			set_page_dirty_lock(cs->pg);
 		}
 		put_page(cs->pg);
-		cs->mapaddr = NULL;
 	}
+	cs->pg = NULL;
 }
 
 /*
@@ -691,7 +684,7 @@ static void fuse_copy_finish(struct fuse
  */
 static int fuse_copy_fill(struct fuse_copy_state *cs)
 {
-	unsigned long offset;
+	struct page *page;
 	int err;
 
 	unlock_request(cs->fc, cs->req);
@@ -706,14 +699,12 @@ static int fuse_copy_fill(struct fuse_co
 
 			BUG_ON(!cs->nr_segs);
 			cs->currbuf = buf;
-			cs->mapaddr = kmap_atomic(buf->page);
+			cs->pg = buf->page;
+			cs->offset = buf->offset;
 			cs->len = buf->len;
-			cs->buf = cs->mapaddr + buf->offset;
 			cs->pipebufs++;
 			cs->nr_segs--;
 		} else {
-			struct page *page;
-
 			if (cs->nr_segs == cs->pipe->buffers)
 				return -EIO;
 
@@ -726,8 +717,8 @@ static int fuse_copy_fill(struct fuse_co
 			buf->len = 0;
 
 			cs->currbuf = buf;
-			cs->mapaddr = kmap_atomic(page);
-			cs->buf = cs->mapaddr;
+			cs->pg = page;
+			cs->offset = 0;
 			cs->len = PAGE_SIZE;
 			cs->pipebufs++;
 			cs->nr_segs++;
@@ -740,14 +731,13 @@ static int fuse_copy_fill(struct fuse_co
 			cs->iov++;
 			cs->nr_segs--;
 		}
-		err = get_user_pages_fast(cs->addr, 1, cs->write, &cs->pg);
+		err = get_user_pages_fast(cs->addr, 1, cs->write, &page);
 		if (err < 0)
 			return err;
 		BUG_ON(err != 1);
-		offset = cs->addr % PAGE_SIZE;
-		cs->mapaddr = kmap_atomic(cs->pg);
-		cs->buf = cs->mapaddr + offset;
-		cs->len = min(PAGE_SIZE - offset, cs->seglen);
+		cs->pg = page;
+		cs->offset = cs->addr % PAGE_SIZE;
+		cs->len = min(PAGE_SIZE - cs->offset, cs->seglen);
 		cs->seglen -= cs->len;
 		cs->addr += cs->len;
 	}
@@ -760,15 +750,20 @@ static int fuse_copy_do(struct fuse_copy
 {
 	unsigned ncpy = min(*size, cs->len);
 	if (val) {
+		void *pgaddr = kmap_atomic(cs->pg);
+		void *buf = pgaddr + cs->offset;
+
 		if (cs->write)
-			memcpy(cs->buf, *val, ncpy);
+			memcpy(buf, *val, ncpy);
 		else
-			memcpy(*val, cs->buf, ncpy);
+			memcpy(*val, buf, ncpy);
+
+		kunmap_atomic(pgaddr);
 		*val += ncpy;
 	}
 	*size -= ncpy;
 	cs->len -= ncpy;
-	cs->buf += ncpy;
+	cs->offset += ncpy;
 	return ncpy;
 }
 
@@ -874,8 +869,8 @@ static int fuse_try_move_page(struct fus
 out_fallback_unlock:
 	unlock_page(newpage);
 out_fallback:
-	cs->mapaddr = kmap_atomic(buf->page);
-	cs->buf = cs->mapaddr + buf->offset;
+	cs->pg = buf->page;
+	cs->offset = buf->offset;
 
 	err = lock_request(cs->fc, cs->req);
 	if (err)

Re: [PATCH] fuse: avoid scheduling while atomic

[Maxim Patlasov]

On 07/04/2014 08:27 PM, Miklos Szeredi wrote:
> Maxim, thanks for the patch.
>
> Here's a reworked one.  While it looks a bit more complicated, its chances of
> being (and remaining) correct are, I think, better, since the region surrounded
> by kmap/kunmap_atomic is trivially non-sleeping.  This patch also removes more
> lines than it adds, as an added bonus.
>
> Please review and test if possible.

The patch looks fine and works well in my tests.

Maxim

>
> Thanks,
> Miklos
>
> ---
>   fs/fuse/dev.c |   51 +++++++++++++++++++++++----------------------------
>   1 file changed, 23 insertions(+), 28 deletions(-)
>
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -643,9 +643,8 @@ struct fuse_copy_state {
>   	unsigned long seglen;
>   	unsigned long addr;
>   	struct page *pg;
> -	void *mapaddr;
> -	void *buf;
>   	unsigned len;
> +	unsigned offset;
>   	unsigned move_pages:1;
>   };
>   
> @@ -666,23 +665,17 @@ static void fuse_copy_finish(struct fuse
>   	if (cs->currbuf) {
>   		struct pipe_buffer *buf = cs->currbuf;
>   
> -		if (!cs->write) {
> -			kunmap_atomic(cs->mapaddr);
> -		} else {
> -			kunmap_atomic(cs->mapaddr);
> +		if (cs->write)
>   			buf->len = PAGE_SIZE - cs->len;
> -		}
>   		cs->currbuf = NULL;
> -		cs->mapaddr = NULL;
> -	} else if (cs->mapaddr) {
> -		kunmap_atomic(cs->mapaddr);
> +	} else if (cs->pg) {
>   		if (cs->write) {
>   			flush_dcache_page(cs->pg);
>   			set_page_dirty_lock(cs->pg);
>   		}
>   		put_page(cs->pg);
> -		cs->mapaddr = NULL;
>   	}
> +	cs->pg = NULL;
>   }
>   
>   /*
> @@ -691,7 +684,7 @@ static void fuse_copy_finish(struct fuse
>    */
>   static int fuse_copy_fill(struct fuse_copy_state *cs)
>   {
> -	unsigned long offset;
> +	struct page *page;
>   	int err;
>   
>   	unlock_request(cs->fc, cs->req);
> @@ -706,14 +699,12 @@ static int fuse_copy_fill(struct fuse_co
>   
>   			BUG_ON(!cs->nr_segs);
>   			cs->currbuf = buf;
> -			cs->mapaddr = kmap_atomic(buf->page);
> +			cs->pg = buf->page;
> +			cs->offset = buf->offset;
>   			cs->len = buf->len;
> -			cs->buf = cs->mapaddr + buf->offset;
>   			cs->pipebufs++;
>   			cs->nr_segs--;
>   		} else {
> -			struct page *page;
> -
>   			if (cs->nr_segs == cs->pipe->buffers)
>   				return -EIO;
>   
> @@ -726,8 +717,8 @@ static int fuse_copy_fill(struct fuse_co
>   			buf->len = 0;
>   
>   			cs->currbuf = buf;
> -			cs->mapaddr = kmap_atomic(page);
> -			cs->buf = cs->mapaddr;
> +			cs->pg = page;
> +			cs->offset = 0;
>   			cs->len = PAGE_SIZE;
>   			cs->pipebufs++;
>   			cs->nr_segs++;
> @@ -740,14 +731,13 @@ static int fuse_copy_fill(struct fuse_co
>   			cs->iov++;
>   			cs->nr_segs--;
>   		}
> -		err = get_user_pages_fast(cs->addr, 1, cs->write, &cs->pg);
> +		err = get_user_pages_fast(cs->addr, 1, cs->write, &page);
>   		if (err < 0)
>   			return err;
>   		BUG_ON(err != 1);
> -		offset = cs->addr % PAGE_SIZE;
> -		cs->mapaddr = kmap_atomic(cs->pg);
> -		cs->buf = cs->mapaddr + offset;
> -		cs->len = min(PAGE_SIZE - offset, cs->seglen);
> +		cs->pg = page;
> +		cs->offset = cs->addr % PAGE_SIZE;
> +		cs->len = min(PAGE_SIZE - cs->offset, cs->seglen);
>   		cs->seglen -= cs->len;
>   		cs->addr += cs->len;
>   	}
> @@ -760,15 +750,20 @@ static int fuse_copy_do(struct fuse_copy
>   {
>   	unsigned ncpy = min(*size, cs->len);
>   	if (val) {
> +		void *pgaddr = kmap_atomic(cs->pg);
> +		void *buf = pgaddr + cs->offset;
> +
>   		if (cs->write)
> -			memcpy(cs->buf, *val, ncpy);
> +			memcpy(buf, *val, ncpy);
>   		else
> -			memcpy(*val, cs->buf, ncpy);
> +			memcpy(*val, buf, ncpy);
> +
> +		kunmap_atomic(pgaddr);
>   		*val += ncpy;
>   	}
>   	*size -= ncpy;
>   	cs->len -= ncpy;
> -	cs->buf += ncpy;
> +	cs->offset += ncpy;
>   	return ncpy;
>   }
>   
> @@ -874,8 +869,8 @@ static int fuse_try_move_page(struct fus
>   out_fallback_unlock:
>   	unlock_page(newpage);
>   out_fallback:
> -	cs->mapaddr = kmap_atomic(buf->page);
> -	cs->buf = cs->mapaddr + buf->offset;
> +	cs->pg = buf->page;
> +	cs->offset = buf->offset;
>   
>   	err = lock_request(cs->fc, cs->req);
>   	if (err)
>