Re: possible deadlock in shmem_fallocate (4)

[Hillf Danton <hdanton@sina.com>]

On Mon, 13 Jul 2020 20:41:11 -0700 Eric Biggers wrote:
> On Tue, Jul 14, 2020 at 11:32:52AM +0800, Hillf Danton wrote:
> > 
> > Add FALLOC_FL_NOBLOCK and on the shmem side try to lock inode upon the
> > new flag. And the overall upside is to keep the current gfp either in
> > the khugepaged context or not.
> > 
> > --- a/include/uapi/linux/falloc.h
> > +++ b/include/uapi/linux/falloc.h
> > @@ -77,4 +77,6 @@
> >   */
> >  #define FALLOC_FL_UNSHARE_RANGE		0x40
> >  
> > +#define FALLOC_FL_NOBLOCK		0x80
> > +
> 
> You can't add a new UAPI flag to fix a kernel-internal problem like this.

Sounds fair, see below.

What the report indicates is a missing PF_MEMALLOC_NOFS and it's
checked on the ashmem side and added as an exception before going
to filesystem. On shmem side, no more than a best effort is paid
on the inteded exception.

--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -437,6 +437,7 @@ static unsigned long
 ashmem_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 {
 	unsigned long freed = 0;
+	bool nofs;
 
 	/* We might recurse into filesystem code, so bail out if necessary */
 	if (!(sc->gfp_mask & __GFP_FS))
@@ -445,6 +446,11 @@ ashmem_shrink_scan(struct shrinker *shri
 	if (!mutex_trylock(&ashmem_mutex))
 		return -1;
 
+	/* enter filesystem with caution: nonblock on locking */
+	nofs = current->flags & PF_MEMALLOC_NOFS;
+	if (!nofs)
+		current->flags |= PF_MEMALLOC_NOFS;
+
 	while (!list_empty(&ashmem_lru_list)) {
 		struct ashmem_range *range =
 			list_first_entry(&ashmem_lru_list, typeof(*range), lru);
@@ -472,6 +478,8 @@ ashmem_shrink_scan(struct shrinker *shri
 	}
 	mutex_unlock(&ashmem_mutex);
 out:
+	if (!nofs)
+		current->flags &= ~PF_MEMALLOC_NOFS;
 	return freed;
 }
 
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2728,7 +2728,12 @@ static long shmem_fallocate(struct file
 	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
 		return -EOPNOTSUPP;
 
-	inode_lock(inode);
+	if (current->flags & PF_MEMALLOC_NOFS) {
+		/* this exception needs a best effort and no more */
+		if (!inode_trylock(inode))
+			return -EBUSY;
+	} else
+		inode_lock(inode);
 
 	if (mode & FALLOC_FL_PUNCH_HOLE) {
 		struct address_space *mapping = file->f_mapping;