Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1309436 > unrolled thread

Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-14 16:40 +0100
    Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-14 17:00 +0100
      Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-14 17:30 +0100
        Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-14 18:00 +0100
          Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-14 18:20 +0100
            Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Tomeu Vizoso <tomeu@tomeuvizoso.net> - 2016-01-14 20:20 +0100
              Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-14 22:10 +0100
                Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-14 22:50 +0100
                  Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-14 23:30 +0100
                    Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-15 00:40 +0100
                    Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Linus Torvalds <torvalds@linux-foundation.org> - 2016-01-15 01:00 +0100
                      Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into  highmem Al Viro <viro@ZenIV.linux.org.uk> - 2016-01-15 01:10 +0100

#1309436 — Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

On Thu, Jan 14, 2016 at 02:22:51PM +0100, Tomeu Vizoso wrote:
> On 9 December 2015 at 06:34, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > From: Al Viro <viro@zeniv.linux.org.uk>
> >
> > kmap() in page_follow_link_light() needed to go - allowing to hold
> > an arbitrary number of kmaps for long is a great way to deadlocking
> > the system.
> >
> > new helper (inode_nohighmem(inode)) needs to be used for pagecache
> > symlinks inodes; done for all in-tree cases.  page_follow_link_light()
> > instrumented to yell about anything missed.
> 
> Hi, starting with with this change, I get this oops when installing
> packages into a rootfs in NFS:

Lovely...  So you somehow getting a highmem page out
                page = read_cache_page(&inode->i_data, 0,
                                        (filler_t *)nfs_symlink_filler, inode);
                if (IS_ERR(page))
                        return ERR_CAST(page);
and that - after
                        inode_nohighmem(inode);
(otherwise you wouldn't get nfs_symlink_inode_operations on that inode).

Could you add
	printk(KERN_ERR "i_data = %p, i_mapping = %p, flags: %lx\n",
		&inode->i_data,
		inode->i_mapping,
		(unsigned long)inode->i_data.flags);
right before the return from nfs_get_link() and see what it prints?

[toc] | [next] | [standalone]

#1309448 — Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

On 14 January 2016 at 16:25, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Jan 14, 2016 at 02:22:51PM +0100, Tomeu Vizoso wrote:
>> On 9 December 2015 at 06:34, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> > From: Al Viro <viro@zeniv.linux.org.uk>
>> >
>> > kmap() in page_follow_link_light() needed to go - allowing to hold
>> > an arbitrary number of kmaps for long is a great way to deadlocking
>> > the system.
>> >
>> > new helper (inode_nohighmem(inode)) needs to be used for pagecache
>> > symlinks inodes; done for all in-tree cases.  page_follow_link_light()
>> > instrumented to yell about anything missed.
>>
>> Hi, starting with with this change, I get this oops when installing
>> packages into a rootfs in NFS:
>
> Lovely...  So you somehow getting a highmem page out
>                 page = read_cache_page(&inode->i_data, 0,
>                                         (filler_t *)nfs_symlink_filler, inode);
>                 if (IS_ERR(page))
>                         return ERR_CAST(page);
> and that - after
>                         inode_nohighmem(inode);
> (otherwise you wouldn't get nfs_symlink_inode_operations on that inode).
>
> Could you add
>         printk(KERN_ERR "i_data = %p, i_mapping = %p, flags: %lx\n",
>                 &inode->i_data,
>                 inode->i_mapping,
>                 (unsigned long)inode->i_data.flags);
> right before the return from nfs_get_link() and see what it prints?

Here it is:

[  170.136956] i_data = ed9c1b04, i_mapping = ed9c1b04, flags: 24200c0
[  170.144567] i_data = ed9de784, i_mapping = ed9de784, flags: 24200c0
[  170.151457] i_data = ed9dec84, i_mapping = ed9dec84, flags: 24200c0
[  170.158358] i_data = ed9c3b84, i_mapping = ed9c3b84, flags: 24200c0
[  170.165253] i_data = ed9d4204, i_mapping = ed9d4204, flags: 24200c0
[  170.172131] i_data = ed9df184, i_mapping = ed9df184, flags: 24200c0
[  170.188804] i_data = eddbce84, i_mapping = eddbce84, flags: 24200c0
[  170.196158] i_data = ec904984, i_mapping = ec904984, flags: 24200c0
[  170.205133] i_data = ec906784, i_mapping = ec906784, flags: 24200c0
[  170.211406] Unable to handle kernel NULL pointer dereference at
virtual address 00000000
[  170.219490] pgd = ee7f0000
[  170.222197] [00000000] *pgd=7b85b835
[  170.225784] Internal error: Oops: 17 [#1] SMP ARM
[  170.230477] Modules linked in:
[  170.233537] CPU: 2 PID: 1 Comm: systemd Not tainted
4.4.0-next-20160114-00005-g6f86169c2250-dirty #3525
[  170.242910] Hardware name: Rockchip (Device Tree)
[  170.247604] task: ee078000 ti: ee062000 task.ti: ee062000
[  170.252996] PC is at strlen+0x0/0x2c
[  170.256565] LR is at readlink_copy+0x24/0x94
[  170.260826] pc : [<c049b138>]    lr : [<c0321160>]    psr: 00000013
[  170.260826] sp : ee063f38  ip : 00000000  fp : ec9066b0
[  170.272281] r10: 001a49f8  r9 : 00000063  r8 : ee063f74
[  170.277494] r7 : 001a4968  r6 : 001a49f8  r5 : 00000000  r4 : 00000063
[  170.284008] r3 : 0000012c  r2 : 00000000  r1 : 00000063  r0 : 00000000
[  170.290522] Flags: nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  170.297642] Control: 10c5387d  Table: 2e7f006a  DAC: 00000051
[  170.303375] Process systemd (pid: 1, stack limit = 0xee062220)
[  170.309196] Stack: (0xee063f38 to 0xee064000)
[  170.313542] 3f20:
    001a49f8 00000063
[  170.321705] 3f40: ffffff9c 001a4968 ee063f74 c0321208 c0321250
eff6b920 ffffffea ffffffea
[  170.329868] 3f60: 00004000 c031c234 00000000 00000000 00000025
00000000 ee102b50 ec874990
[  170.338030] 3f80: 5697c41b 001a49f8 00000064 00000063 0000014c
c0210e84 ee062000 00000000
[  170.346193] 3fa0: 001a4968 c0210cc0 001a49f8 00000064 ffffff9c
001a4968 001a49f8 00000063
[  170.354355] 3fc0: 001a49f8 00000064 00000063 0000014c bec450b8
001c1094 001d0933 001a4968
[  170.362518] 3fe0: 0000014c bec45094 b6f2af7b b6eb88e6 20000030
ffffff9c ffffffff ffeffffe
[  170.370683] [<c049b138>] (strlen) from [<c0321160>] (readlink_copy+0x24/0x94)
[  170.377806] [<c0321160>] (readlink_copy) from [<c0321208>]
(generic_readlink+0x38/0x80)
[  170.385796] [<c0321208>] (generic_readlink) from [<c031c234>]
(SyS_readlinkat+0x98/0xe0)
[  170.393874] [<c031c234>] (SyS_readlinkat) from [<c0210cc0>]
(ret_fast_syscall+0x0/0x3c)
[  170.401863] Code: e7d23003 e3130020 1afffffb e12fff1e (e5d02000)
[  170.407965] ---[ end trace 87f95166dedbabb0 ]---

Full log at https://lava.collabora.co.uk/scheduler/job/127626/log_file

Regards,

Tomeu

[toc] | [prev] | [next] | [standalone]

#1309464

On Thu, Jan 14, 2016 at 04:58:48PM +0100, Tomeu Vizoso wrote:
> > Could you add
> >         printk(KERN_ERR "i_data = %p, i_mapping = %p, flags: %lx\n",
> >                 &inode->i_data,
> >                 inode->i_mapping,
> >                 (unsigned long)inode->i_data.flags);
> > right before the return from nfs_get_link() and see what it prints?
> 
> Here it is:
> 
> [  170.136956] i_data = ed9c1b04, i_mapping = ed9c1b04, flags: 24200c0
> [  170.144567] i_data = ed9de784, i_mapping = ed9de784, flags: 24200c0
> [  170.151457] i_data = ed9dec84, i_mapping = ed9dec84, flags: 24200c0
> [  170.158358] i_data = ed9c3b84, i_mapping = ed9c3b84, flags: 24200c0
> [  170.165253] i_data = ed9d4204, i_mapping = ed9d4204, flags: 24200c0
> [  170.172131] i_data = ed9df184, i_mapping = ed9df184, flags: 24200c0
> [  170.188804] i_data = eddbce84, i_mapping = eddbce84, flags: 24200c0
> [  170.196158] i_data = ec904984, i_mapping = ec904984, flags: 24200c0
> [  170.205133] i_data = ec906784, i_mapping = ec906784, flags: 24200c0

Aha.  So ->i_data vs. ->i_mapping is irrelevant (as it ought to be here)
and inode_nohighmem() should've acted on the address_space we are hitting
here.  What do we have in flags...  ___GFP_IO | ___GFP_FS | ___GFP_HARDWALL |
___GFP_DIRECT_RECLAIM | ___GFP_KSWAPD_RECLAIM.  IOW, normal GFP_USER, no
__GFP_HIGHMEM in sight.

So either we have a highmem page somehow ending up in i_data before we
set the flags, or __page_cache_alloc() done by read_cache_page() returns
us a highmem page on GFP_USER | __GFP_COLD (or I'm misreading the things
completely)...

Could you slap
	printk(KERN_ERR "inode: %p, pages: %ld\n",
		inode, inode->i_data.nrpages);
before that read_cache_page() in nfs_get_link() and
	printk(KERN_ERR "page_address: %p\n", page_address(page));
right before the return?

[toc] | [prev] | [next] | [standalone]

#1309486 — Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

On 14 January 2016 at 17:23, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Jan 14, 2016 at 04:58:48PM +0100, Tomeu Vizoso wrote:
>> > Could you add
>> >         printk(KERN_ERR "i_data = %p, i_mapping = %p, flags: %lx\n",
>> >                 &inode->i_data,
>> >                 inode->i_mapping,
>> >                 (unsigned long)inode->i_data.flags);
>> > right before the return from nfs_get_link() and see what it prints?
>>
>> Here it is:
>>
>> [  170.136956] i_data = ed9c1b04, i_mapping = ed9c1b04, flags: 24200c0
>> [  170.144567] i_data = ed9de784, i_mapping = ed9de784, flags: 24200c0
>> [  170.151457] i_data = ed9dec84, i_mapping = ed9dec84, flags: 24200c0
>> [  170.158358] i_data = ed9c3b84, i_mapping = ed9c3b84, flags: 24200c0
>> [  170.165253] i_data = ed9d4204, i_mapping = ed9d4204, flags: 24200c0
>> [  170.172131] i_data = ed9df184, i_mapping = ed9df184, flags: 24200c0
>> [  170.188804] i_data = eddbce84, i_mapping = eddbce84, flags: 24200c0
>> [  170.196158] i_data = ec904984, i_mapping = ec904984, flags: 24200c0
>> [  170.205133] i_data = ec906784, i_mapping = ec906784, flags: 24200c0
>
> Aha.  So ->i_data vs. ->i_mapping is irrelevant (as it ought to be here)
> and inode_nohighmem() should've acted on the address_space we are hitting
> here.  What do we have in flags...  ___GFP_IO | ___GFP_FS | ___GFP_HARDWALL |
> ___GFP_DIRECT_RECLAIM | ___GFP_KSWAPD_RECLAIM.  IOW, normal GFP_USER, no
> __GFP_HIGHMEM in sight.
>
> So either we have a highmem page somehow ending up in i_data before we
> set the flags, or __page_cache_alloc() done by read_cache_page() returns
> us a highmem page on GFP_USER | __GFP_COLD (or I'm misreading the things
> completely)...
>
> Could you slap
>         printk(KERN_ERR "inode: %p, pages: %ld\n",
>                 inode, inode->i_data.nrpages);
> before that read_cache_page() in nfs_get_link() and
>         printk(KERN_ERR "page_address: %p\n", page_address(page));
> right before the return?

Here it is:

[  170.715356] inode: ec8c30b0, pages: 1
[  170.719014] page_address:   (null)

https://lava.collabora.co.uk/scheduler/job/127698/log_file

Regards,

Tomeu

[toc] | [prev] | [next] | [standalone]

#1309497

On Thu, Jan 14, 2016 at 05:57:42PM +0100, Tomeu Vizoso wrote:
> Here it is:
> 
> [  170.715356] inode: ec8c30b0, pages: 1
> [  170.719014] page_address:   (null)
> 
> https://lava.collabora.co.uk/scheduler/job/127698/log_file

	Lovely...  And that looks like the first time that inode hits
nfs_get_link().  Ho-hum...

	Could you add WARN_ON(inode->i_mapping.nrpages) in inode_nohighmem()
and see if that triggers?  It really shouldn't (we hit it after iget5_locked()
in nfs_fhget() has returned us a new inode, and there shouldn't be a chance
for any pages to get in there between struct inode allocation and that
point), but then neither should highmem pages be added to address_space
without __GFP_HIGHMEM in ->flags...

[toc] | [prev] | [next] | [standalone]

#1309587 — Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

On 14 January 2016 at 18:13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Jan 14, 2016 at 05:57:42PM +0100, Tomeu Vizoso wrote:
>> Here it is:
>>
>> [  170.715356] inode: ec8c30b0, pages: 1
>> [  170.719014] page_address:   (null)
>>
>> https://lava.collabora.co.uk/scheduler/job/127698/log_file
>
>         Lovely...  And that looks like the first time that inode hits
> nfs_get_link().  Ho-hum...
>
>         Could you add WARN_ON(inode->i_mapping.nrpages) in inode_nohighmem()
> and see if that triggers?  It really shouldn't (we hit it after iget5_locked()

Indeed :(

https://lava.collabora.co.uk/scheduler/job/127782/log_file

diff --git a/fs/inode.c b/fs/inode.c
index 1e6dd388ba7f..b957744376e8 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2031,6 +2031,7 @@ EXPORT_SYMBOL(inode_set_flags);

 void inode_nohighmem(struct inode *inode)
 {
+       WARN_ON(inode->i_mapping->nrpages);
        mapping_set_gfp_mask(inode->i_mapping, GFP_USER);
 }
 EXPORT_SYMBOL(inode_nohighmem);

I have ran out of time for today, but tomorrow can do more tests if needed.

Regards,

Tomeu

> in nfs_fhget() has returned us a new inode, and there shouldn't be a chance
> for any pages to get in there between struct inode allocation and that
> point), but then neither should highmem pages be added to address_space
> without __GFP_HIGHMEM in ->flags...

[toc] | [prev] | [next] | [standalone]

#1309651

On Thu, Jan 14, 2016 at 08:15:57PM +0100, Tomeu Vizoso wrote:
> On 14 January 2016 at 18:13, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Thu, Jan 14, 2016 at 05:57:42PM +0100, Tomeu Vizoso wrote:
> >> Here it is:
> >>
> >> [  170.715356] inode: ec8c30b0, pages: 1
> >> [  170.719014] page_address:   (null)
> >>
> >> https://lava.collabora.co.uk/scheduler/job/127698/log_file
> >
> >         Lovely...  And that looks like the first time that inode hits
> > nfs_get_link().  Ho-hum...
> >
> >         Could you add WARN_ON(inode->i_mapping.nrpages) in inode_nohighmem()
> > and see if that triggers?  It really shouldn't (we hit it after iget5_locked()
> 
> Indeed :(
> 
> https://lava.collabora.co.uk/scheduler/job/127782/log_file

OK...  Unless I'm misreading that, we have
	* inode->i_data.flags set to GFP_USER, with no pages present in
there.
	* at some later point nfs_get_link() is called on that inode
(for the first time) and sees a page with logical offset 0 already present
in there, that page being a highmem one.

That would certainly suffice for the things to blow up...

Let's try this: in the beginning of __add_to_page_cache_locked() add
	VM_BUG_ON_PAGE(PageHighMem(page) & !(mapping->flags & __GFP_HIGHMEM),
			page);
and see if that triggers.

<pokes around>

Arrrgh.  Try this:

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index ce5a218..8a05309 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1894,15 +1894,14 @@ int nfs_symlink(struct inode *dir, struct dentry *dentry, const char *symname)
 	attr.ia_mode = S_IFLNK | S_IRWXUGO;
 	attr.ia_valid = ATTR_MODE;
 
-	page = alloc_page(GFP_HIGHUSER);
+	page = alloc_page(GFP_USER);
 	if (!page)
 		return -ENOMEM;
 
-	kaddr = kmap_atomic(page);
+	kaddr = page_address(page);
 	memcpy(kaddr, symname, pathlen);
 	if (pathlen < PAGE_SIZE)
 		memset(kaddr + pathlen, 0, PAGE_SIZE - pathlen);
-	kunmap_atomic(kaddr);
 
 	trace_nfs_symlink_enter(dir, dentry);
 	error = NFS_PROTO(dir)->symlink(dir, dentry, page, pathlen, &attr);

[toc] | [prev] | [next] | [standalone]

#1309676 — Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

On Thu, Jan 14, 2016 at 1:02 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> Arrrgh.  Try this:

Yeah, that would do it.

Al, did you check any other filesystems do this?

Also, I'm wondering if we should perhaps revert the "don't use
highmem". Do we actually have examples of running out of kmaps? Do we
care?

                Linus

[toc] | [prev] | [next] | [standalone]

#1309711

On Thu, Jan 14, 2016 at 01:40:32PM -0800, Linus Torvalds wrote:
> On Thu, Jan 14, 2016 at 1:02 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > Arrrgh.  Try this:
> 
> Yeah, that would do it.
> 
> Al, did you check any other filesystems do this?

There's one more turd like that - shmem should've done inode_nohighmem()
a bit earlier in shmem_symlink().  The rest is OK.

> Also, I'm wondering if we should perhaps revert the "don't use
> highmem". Do we actually have examples of running out of kmaps? Do we
> care?

For one thing, we'll lose RCU ->get_link() for those.  For another... yes,
it was a nasty bug (I missed the possibility that filesystem might seed
the page cache on ->symlink() directly and use a highmem page - mea culpa),
but we can easily catch it at runtime.  We really shouldn't put highmem
pages into address_space without __GFP_HIGHMEM, and catching those in
__add_to_page_cache_locked() isn't costly.

Anyway, mm/shmem.c bit follows.  With that + NFS one we ought to be OK
wrt that class of bogosities.  I'll write the bits for
Documentation/filesystems/porting (basically, "if you preseed the pagecache
at ->symlink() time, don't put highmem pages there; page_symlink() will
take care of that, provided that inode_nohighmem() is called first") and
push the combined patch to #for-linus.

diff --git a/mm/shmem.c b/mm/shmem.c
index 5813b7f..642471b 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2469,6 +2469,7 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
 		inode->i_op = &shmem_short_symlink_operations;
 		inode->i_link = info->symlink;
 	} else {
+		inode_nohighmem(inode);
 		error = shmem_getpage(inode, 0, &page, SGP_WRITE, NULL);
 		if (error) {
 			iput(inode);
@@ -2476,7 +2477,6 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
 		}
 		inode->i_mapping->a_ops = &shmem_aops;
 		inode->i_op = &shmem_symlink_inode_operations;
-		inode_nohighmem(inode);
 		memcpy(page_address(page), symname, len);
 		SetPageUptodate(page);
 		set_page_dirty(page);

[toc] | [prev] | [next] | [standalone]

#1309758

On Thu, Jan 14, 2016 at 10:25:44PM +0000, Al Viro wrote:

> Anyway, mm/shmem.c bit follows.  With that + NFS one we ought to be OK
> wrt that class of bogosities.  I'll write the bits for
> Documentation/filesystems/porting (basically, "if you preseed the pagecache
> at ->symlink() time, don't put highmem pages there; page_symlink() will
> take care of that, provided that inode_nohighmem() is called first") and
> push the combined patch to #for-linus.

Done and pushed.  Please, pull from
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git

Shortlog:
Al Viro (1):
      Make sure that highmem pages are not added to symlink page cache

Diffstat:
 Documentation/filesystems/porting | 6 +++++-
 fs/nfs/dir.c                      | 5 ++---
 mm/shmem.c                        | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

[toc] | [prev] | [next] | [standalone]

#1309771 — Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

On Thu, Jan 14, 2016 at 2:25 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> For one thing, we'll lose RCU ->get_link() for those.

Why couldn't we just do that in the RCU walker? kmap should be fine..

That said, as long as you think it's ok now, I guess I don't care.
Having some sanity testing in __add_to_page_cache_locked might be a
good safety net.

             Linus

[toc] | [prev] | [next] | [standalone]

#1309775

On Thu, Jan 14, 2016 at 03:58:22PM -0800, Linus Torvalds wrote:
> On Thu, Jan 14, 2016 at 2:25 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > For one thing, we'll lose RCU ->get_link() for those.
> 
> Why couldn't we just do that in the RCU walker? kmap should be fine..

In map_new_virtual():
                        __set_current_state(TASK_UNINTERRUPTIBLE);
                        add_wait_queue(pkmap_map_wait, &wait);
                        unlock_kmap();
                        schedule();
                        remove_wait_queue(pkmap_map_wait, &wait);
                        lock_kmap();
IOW, not in RCU mode ;-/

> That said, as long as you think it's ok now, I guess I don't care.
> Having some sanity testing in __add_to_page_cache_locked might be a
> good safety net.

That's better as a separate commit, IMO.  The thing I'm not sure about is
whether we want a BUG() in there - VM_WARN_ON(), perhaps?  OTOH, we do
have VM_BUG_ON_PAGE() in the same place already...

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web