Skip to contentSkip to conversation

drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

PATCH In review Fengguang Wu 3 participants 7 messages Nov 30, 2012 – Dec 2, 2012 1 message not in the archive Memory management

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 50b79f52.Rxsdi7iwHf+1mkK5%[email protected]

Fengguang Wu

author
#1 lore raw

PATCH 341/499 drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

Hi Rafael,

[Julia and me think that this coccinelle warning is worth reporting.]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 5f1da4063294480b3fabcee554f976565dec54b5 [341/499] virtio_balloon: introduce migration primitives to balloon pages

+ drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

vim +157 drivers/virtio/virtio_balloon.c

6b35e407 Rusty Russell      2008-02-04  130  {
5f1da406 Rafael Aquini      2012-11-09  131  	struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
5f1da406 Rafael Aquini      2012-11-09  132  
6b35e407 Rusty Russell      2008-02-04  133  	/* We can only do one array worth at a time. */
6b35e407 Rusty Russell      2008-02-04  134  	num = min(num, ARRAY_SIZE(vb->pfns));
6b35e407 Rusty Russell      2008-02-04  135  
5f1da406 Rafael Aquini      2012-11-09 @136  	mutex_lock(&vb->balloon_lock);
3ccc9372 Michael S. Tsirkin 2012-04-12  137  	for (vb->num_pfns = 0; vb->num_pfns < num;
3ccc9372 Michael S. Tsirkin 2012-04-12  138  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
5f1da406 Rafael Aquini      2012-11-09  139  		struct page *page = balloon_page_enqueue(vb_dev_info);
5f1da406 Rafael Aquini      2012-11-09  140  
6b35e407 Rusty Russell      2008-02-04  141  		if (!page) {
6b35e407 Rusty Russell      2008-02-04  142  			if (printk_ratelimit())
6b35e407 Rusty Russell      2008-02-04  143  				dev_printk(KERN_INFO, &vb->vdev->dev,
6b35e407 Rusty Russell      2008-02-04  144  					   "Out of puff! Can't get %zu pages\n",
5f1da406 Rafael Aquini      2012-11-09  145  					   VIRTIO_BALLOON_PAGES_PER_PAGE);
6b35e407 Rusty Russell      2008-02-04  146  			/* Sleep for at least 1/5 of a second before retry. */
6b35e407 Rusty Russell      2008-02-04  147  			msleep(200);
6b35e407 Rusty Russell      2008-02-04  148  			break;
6b35e407 Rusty Russell      2008-02-04  149  		}
3ccc9372 Michael S. Tsirkin 2012-04-12  150  		set_page_pfns(vb->pfns + vb->num_pfns, page);
3ccc9372 Michael S. Tsirkin 2012-04-12  151  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
6b35e407 Rusty Russell      2008-02-04  152  		totalram_pages--;
6b35e407 Rusty Russell      2008-02-04  153  	}
6b35e407 Rusty Russell      2008-02-04  154  
6b35e407 Rusty Russell      2008-02-04  155  	/* Didn't get any?  Oh well. */
6b35e407 Rusty Russell      2008-02-04  156  	if (vb->num_pfns == 0)
6b35e407 Rusty Russell      2008-02-04 @157  		return;
6b35e407 Rusty Russell      2008-02-04  158  
6b35e407 Rusty Russell      2008-02-04  159  	tell_host(vb, vb->inflate_vq);
5f1da406 Rafael Aquini      2012-11-09  160  	mutex_unlock(&vb->balloon_lock);

---
0-DAY kernel build testing backend Open Source Technology Center
Fengguang Wu, Yuanhan Liu Intel Corporation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#2 lore raw
Quoted 35 lines · On Fri, 30 Nov 2012 08:28:48 +0800 Fengguang Wu <[email protected]> wrote:

Hi Rafael,

[Julia and me think that this coccinelle warning is worth reporting.]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 5f1da4063294480b3fabcee554f976565dec54b5 [341/499] virtio_balloon: introduce migration primitives to balloon pages

+ drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

vim +157 drivers/virtio/virtio_balloon.c

6b35e407 Rusty Russell      2008-02-04  130  {
5f1da406 Rafael Aquini      2012-11-09  131  	struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
5f1da406 Rafael Aquini      2012-11-09  132  
6b35e407 Rusty Russell      2008-02-04  133  	/* We can only do one array worth at a time. */
6b35e407 Rusty Russell      2008-02-04  134  	num = min(num, ARRAY_SIZE(vb->pfns));
6b35e407 Rusty Russell      2008-02-04  135  
5f1da406 Rafael Aquini      2012-11-09 @136  	mutex_lock(&vb->balloon_lock);
3ccc9372 Michael S. Tsirkin 2012-04-12  137  	for (vb->num_pfns = 0; vb->num_pfns < num;
3ccc9372 Michael S. Tsirkin 2012-04-12  138  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
5f1da406 Rafael Aquini      2012-11-09  139  		struct page *page = balloon_page_enqueue(vb_dev_info);
5f1da406 Rafael Aquini      2012-11-09  140  
6b35e407 Rusty Russell      2008-02-04  141  		if (!page) {
6b35e407 Rusty Russell      2008-02-04  142  			if (printk_ratelimit())
6b35e407 Rusty Russell      2008-02-04  143  				dev_printk(KERN_INFO, &vb->vdev->dev,
6b35e407 Rusty Russell      2008-02-04  144  					   "Out of puff! Can't get %zu pages\n",
5f1da406 Rafael Aquini      2012-11-09  145  					   VIRTIO_BALLOON_PAGES_PER_PAGE);
6b35e407 Rusty Russell      2008-02-04  146  			/* Sleep for at least 1/5 of a second before retry. */
6b35e407 Rusty Russell      2008-02-04  147  			msleep(200);
6b35e407 Rusty Russell      2008-02-04  148  			break;
6b35e407 Rusty Russell      2008-02-04  149  		}
3ccc9372 Michael S. Tsirkin 2012-04-12  150  		set_page_pfns(vb->pfns + vb->num_pfns, page);
3ccc9372 Michael S. Tsirkin 2012-04-12  151  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
6b35e407 Rusty Russell      2008-02-04  152  		totalram_pages--;
Quoted 8 lines
6b35e407 Rusty Russell      2008-02-04  153  	}
6b35e407 Rusty Russell      2008-02-04  154  
6b35e407 Rusty Russell      2008-02-04  155  	/* Didn't get any?  Oh well. */
6b35e407 Rusty Russell      2008-02-04  156  	if (vb->num_pfns == 0)
6b35e407 Rusty Russell      2008-02-04 @157  		return;
6b35e407 Rusty Russell      2008-02-04  158  
6b35e407 Rusty Russell      2008-02-04  159  	tell_host(vb, vb->inflate_vq);
5f1da406 Rafael Aquini      2012-11-09  160  	mutex_unlock(&vb->balloon_lock);

This bug was fixed by

virtio_balloon-introduce-migration-primitives-to-balloon-pages.patch
this one -> virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix.patch
virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix.patch virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix-fix.patch

I'm surprised this sort of thing hasn't occurred before now - I'd assumed that your bisection system knew about the followup patches and skipped them.

I wonder how hard this is to do. My naming scheme is pretty simple:

foo-bar-zot.patch
foo-bar-zot-[some-other-text].patch
foo-bar-zot-[some-different-other-text].patch

But those are the local-to-mm filenames. The Subject: field in the linux-next commit is sometimes quite different from the original filename. But I can stop doing that, and ensure that the Subject:
matches the filename for fixup patches.

That means that your are-there-any-fixup-patches logic would need to do this:

- Look at the patch title, For example, "virtio_balloon: introduce migration primitives to balloon pages".

- Mangle that title into an akpm patch filename. The dopey script I use to do this is below. In this example it will yield virtio_balloon-introduce-migration-primitives-to-balloon-pages

- Look at the Subject: field of the following patches. If they start with virtio_balloon-introduce-migration-primitives-to-balloon-pages then skip over them.

#!/bin/sh
. patchfns 2>/dev/null ||
. /usr/lib/patch-scripts/patchfns 2>/dev/null ||
. $PATCHSCRIPTS_LIBDIR/patchfns 2>/dev/null ||
{
echo "Impossible to find my library 'patchfns'." echo "Check your install, or go to the right directory" exit 1
}

line="$1"
if stripit $line 2>/dev/null > /dev/null
then

	f=$(stripit $line)
	if [ -e txt/$f.txt ]
	then
		line="$(grep "^Subject: " txt/$f.txt | head -n 1)"
		line=$(echo $line | sed -e "s/^Subject: //")
	fi

fi

line=$(echo "$line" | tr 'A-Z' 'a-z')
line=$(echo "$line" | sed -e 's/^subject:[ 	]*//')
line=$(echo "$line" | sed -e 's/^fw:[ 	]*//')
line=$(echo "$line" | sed -e 's/(fwd)//g')
line=$(echo "$line" | sed -e 's/^fwd:[ 	]*//')
line=$(echo "$line" | sed -e 's/^aw:[ 	]*//')
line=$(echo "$line" | sed -e 's/^re:[ 	]*//')
line=$(echo "$line" | sed -e 's/^patch//')
line=$(echo "$line" | sed -e "s/['\(\)\<\>\{\}\,\.\\]//g")
line=$(echo "$line" | sed -e "s/[\#\*\&\+\^\!\~\`\|\?\;]//g")
line=$(echo "$line" | sed -e "s/[\$]//g")
line=$(echo "$line" | sed -e 's/"//g')
line=$(echo "$line" | sed -e 's/^[-]*//g')
line=$(echo "$line" | sed -e 's/\[[^]]*\]//g')
line=$(echo "$line" | sed -e 's/[ 	]*\[patch\][	]*//')
line=$(echo "$line" | sed -e 's/\[//g')
line=$(echo "$line" | sed -e 's/\]//g')
line=$(echo "$line" | sed -e 's/^[ 	]*//')
line=$(echo "$line" | sed -e 's/ -/-/g')
line=$(echo "$line" | sed -e 's/@/-/g')
line=$(echo "$line" | sed -e 's/- /-/g')
line=$(echo "$line" | sed -e 's/[ 	][ 	]*/-/g')
line=$(echo "$line" | sed -e 's,/,-,g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/-$//g')
line=$(echo "$line" | sed -e 's/^-//g')
line=$(echo "$line" | sed -e 's/:/-/g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/^[-]//')
line=$(echo "$line" | sed -e 's/[-]$//')
line=$(echo "$line" | sed -e 's/[-]$//')
line=$(echo "$line" | sed -e 's/[-]$//')

echo "$line"

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

author
#3 lore raw
Quoted 47 lines · Andrew Morton
Quoted 35 lines · On Fri, 30 Nov 2012 08:28:48 +0800 Fengguang Wu <[email protected]> wrote:

Hi Rafael,

[Julia and me think that this coccinelle warning is worth reporting.]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 5f1da4063294480b3fabcee554f976565dec54b5 [341/499] virtio_balloon: introduce migration primitives to balloon pages

+ drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

vim +157 drivers/virtio/virtio_balloon.c

6b35e407 Rusty Russell      2008-02-04  130  {
5f1da406 Rafael Aquini      2012-11-09  131  	struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
5f1da406 Rafael Aquini      2012-11-09  132  
6b35e407 Rusty Russell      2008-02-04  133  	/* We can only do one array worth at a time. */
6b35e407 Rusty Russell      2008-02-04  134  	num = min(num, ARRAY_SIZE(vb->pfns));
6b35e407 Rusty Russell      2008-02-04  135  
5f1da406 Rafael Aquini      2012-11-09 @136  	mutex_lock(&vb->balloon_lock);
3ccc9372 Michael S. Tsirkin 2012-04-12  137  	for (vb->num_pfns = 0; vb->num_pfns < num;
3ccc9372 Michael S. Tsirkin 2012-04-12  138  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
5f1da406 Rafael Aquini      2012-11-09  139  		struct page *page = balloon_page_enqueue(vb_dev_info);
5f1da406 Rafael Aquini      2012-11-09  140  
6b35e407 Rusty Russell      2008-02-04  141  		if (!page) {
6b35e407 Rusty Russell      2008-02-04  142  			if (printk_ratelimit())
6b35e407 Rusty Russell      2008-02-04  143  				dev_printk(KERN_INFO, &vb->vdev->dev,
6b35e407 Rusty Russell      2008-02-04  144  					   "Out of puff! Can't get %zu pages\n",
5f1da406 Rafael Aquini      2012-11-09  145  					   VIRTIO_BALLOON_PAGES_PER_PAGE);
6b35e407 Rusty Russell      2008-02-04  146  			/* Sleep for at least 1/5 of a second before retry. */
6b35e407 Rusty Russell      2008-02-04  147  			msleep(200);
6b35e407 Rusty Russell      2008-02-04  148  			break;
6b35e407 Rusty Russell      2008-02-04  149  		}
3ccc9372 Michael S. Tsirkin 2012-04-12  150  		set_page_pfns(vb->pfns + vb->num_pfns, page);
3ccc9372 Michael S. Tsirkin 2012-04-12  151  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
6b35e407 Rusty Russell      2008-02-04  152  		totalram_pages--;
Quoted 8 lines
6b35e407 Rusty Russell      2008-02-04  153  	}
6b35e407 Rusty Russell      2008-02-04  154  
6b35e407 Rusty Russell      2008-02-04  155  	/* Didn't get any?  Oh well. */
6b35e407 Rusty Russell      2008-02-04  156  	if (vb->num_pfns == 0)
6b35e407 Rusty Russell      2008-02-04 @157  		return;
6b35e407 Rusty Russell      2008-02-04  158  
6b35e407 Rusty Russell      2008-02-04  159  	tell_host(vb, vb->inflate_vq);
5f1da406 Rafael Aquini      2012-11-09  160  	mutex_unlock(&vb->balloon_lock);
Quoted 6 lines

This bug was fixed by

virtio_balloon-introduce-migration-primitives-to-balloon-pages.patch
this one -> virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix.patch
virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix.patch virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix-fix.patch

Michal: your since-3.6 branch somehow missed that followup fix...

Quoted 3 lines

I'm surprised this sort of thing hasn't occurred before now - I'd assumed that your bisection system knew about the followup patches and skipped them.

Sure. I've been detecting and skipping your follow up fixes in the "commit by commit" build test mode and the logic has been working well, which is basically:

        [[ $(git_commit_subject $commit) =~ (fix|build|error|warning) ]] && return 0
        [[ $(git_commit_changelog $commit) =~ (build|compile|gcc|sparse|smatch|coccinelle|error|warning) ]] && return 0

Note that it's perfectly fine for the above logic to have false positives. If ever a commit (which introduces a build error) is skipped due to being falsely recognized as some followup fix, the build error will still be detected in the next commit (or eventually, in the HEAD which will always be build tested). bisect will then be auto started and locate the error back to the skipped commit.

Note that the build system also runs a "check HEAD + bisect errors" mode for some cases, in which it's not necessary to detect possible fixup patches during bisect. Because if ever there is a working fix, the error won't be able to show up in the branch HEAD at all, thus bisect won't be started in the first place.

Quoted 10 lines

I wonder how hard this is to do. My naming scheme is pretty simple:

foo-bar-zot.patch
foo-bar-zot-[some-other-text].patch
foo-bar-zot-[some-different-other-text].patch

But those are the local-to-mm filenames. The Subject: field in the linux-next commit is sometimes quite different from the original filename. But I can stop doing that, and ensure that the Subject:
matches the filename for fixup patches.

I simply picked up the keyword "fix" in your typical name scheme "XXXXXX-fix" ;-)

Quoted 6 lines

That means that your are-there-any-fixup-patches logic would need to do this:

- Look at the patch title, For example, "virtio_balloon: introduce migration primitives to balloon pages".

Quoted 8 lines

- Mangle that title into an akpm patch filename. The dopey script I use to do this is below. In this example it will yield virtio_balloon-introduce-migration-primitives-to-balloon-pages

- Look at the Subject: field of the following patches. If they start with virtio_balloon-introduce-migration-primitives-to-balloon-pages then skip over them.

Thanks for sharing the code and howto! It won't be hard for me to follow that rule, however it seems a bit more straightforward to detect the "fix" magic keyword in the subject line?

Thanks,
Fengguang

Quoted 56 lines

#!/bin/sh
. patchfns 2>/dev/null ||
. /usr/lib/patch-scripts/patchfns 2>/dev/null ||
. $PATCHSCRIPTS_LIBDIR/patchfns 2>/dev/null ||
{
echo "Impossible to find my library 'patchfns'." echo "Check your install, or go to the right directory" exit 1
}

line="$1"
if stripit $line 2>/dev/null > /dev/null
then

	f=$(stripit $line)
	if [ -e txt/$f.txt ]
	then
		line="$(grep "^Subject: " txt/$f.txt | head -n 1)"
		line=$(echo $line | sed -e "s/^Subject: //")
	fi

fi

line=$(echo "$line" | tr 'A-Z' 'a-z')
line=$(echo "$line" | sed -e 's/^subject:[ 	]*//')
line=$(echo "$line" | sed -e 's/^fw:[ 	]*//')
line=$(echo "$line" | sed -e 's/(fwd)//g')
line=$(echo "$line" | sed -e 's/^fwd:[ 	]*//')
line=$(echo "$line" | sed -e 's/^aw:[ 	]*//')
line=$(echo "$line" | sed -e 's/^re:[ 	]*//')
line=$(echo "$line" | sed -e 's/^patch//')
line=$(echo "$line" | sed -e "s/['\(\)\<\>\{\}\,\.\\]//g")
line=$(echo "$line" | sed -e "s/[\#\*\&\+\^\!\~\`\|\?\;]//g")
line=$(echo "$line" | sed -e "s/[\$]//g")
line=$(echo "$line" | sed -e 's/"//g')
line=$(echo "$line" | sed -e 's/^[-]*//g')
line=$(echo "$line" | sed -e 's/\[[^]]*\]//g')
line=$(echo "$line" | sed -e 's/[ 	]*\[patch\][	]*//')
line=$(echo "$line" | sed -e 's/\[//g')
line=$(echo "$line" | sed -e 's/\]//g')
line=$(echo "$line" | sed -e 's/^[ 	]*//')
line=$(echo "$line" | sed -e 's/ -/-/g')
line=$(echo "$line" | sed -e 's/@/-/g')
line=$(echo "$line" | sed -e 's/- /-/g')
line=$(echo "$line" | sed -e 's/[ 	][ 	]*/-/g')
line=$(echo "$line" | sed -e 's,/,-,g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/-$//g')
line=$(echo "$line" | sed -e 's/^-//g')
line=$(echo "$line" | sed -e 's/:/-/g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/--/-/g')
line=$(echo "$line" | sed -e 's/^[-]//')
Quoted 7 lines
line=$(echo "$line" | sed -e 's/[-]$//')
line=$(echo "$line" | sed -e 's/[-]$//')
line=$(echo "$line" | sed -e 's/[-]$//')

echo "$line"

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

maintainer
#4 lore raw
Quoted 3 lines · On Fri, 30 Nov 2012 10:00:15 +0800 Fengguang Wu <[email protected]> wrote:

Thanks for sharing the code and howto! It won't be hard for me to follow that rule, however it seems a bit more straightforward to detect the "fix" magic keyword in the subject line?

OK, including "fix" is easy ;)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#5 lore raw
In reply to Fengguang Wu
Quoted 48 lines · Wu Fengguang
Quoted 47 lines · Andrew Morton
Quoted 43 lines · On Fri, 30 Nov 2012 08:28:48 +0800 Fengguang Wu <[email protected]> wrote:

Hi Rafael,

[Julia and me think that this coccinelle warning is worth reporting.]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 5f1da4063294480b3fabcee554f976565dec54b5 [341/499] virtio_balloon: introduce migration primitives to balloon pages

+ drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

vim +157 drivers/virtio/virtio_balloon.c

6b35e407 Rusty Russell      2008-02-04  130  {
5f1da406 Rafael Aquini      2012-11-09  131  	struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
5f1da406 Rafael Aquini      2012-11-09  132  
6b35e407 Rusty Russell      2008-02-04  133  	/* We can only do one array worth at a time. */
6b35e407 Rusty Russell      2008-02-04  134  	num = min(num, ARRAY_SIZE(vb->pfns));
6b35e407 Rusty Russell      2008-02-04  135  
5f1da406 Rafael Aquini      2012-11-09 @136  	mutex_lock(&vb->balloon_lock);
3ccc9372 Michael S. Tsirkin 2012-04-12  137  	for (vb->num_pfns = 0; vb->num_pfns < num;
3ccc9372 Michael S. Tsirkin 2012-04-12  138  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
5f1da406 Rafael Aquini      2012-11-09  139  		struct page *page = balloon_page_enqueue(vb_dev_info);
5f1da406 Rafael Aquini      2012-11-09  140  
6b35e407 Rusty Russell      2008-02-04  141  		if (!page) {
6b35e407 Rusty Russell      2008-02-04  142  			if (printk_ratelimit())
6b35e407 Rusty Russell      2008-02-04  143  				dev_printk(KERN_INFO, &vb->vdev->dev,
6b35e407 Rusty Russell      2008-02-04  144  					   "Out of puff! Can't get %zu pages\n",
5f1da406 Rafael Aquini      2012-11-09  145  					   VIRTIO_BALLOON_PAGES_PER_PAGE);
6b35e407 Rusty Russell      2008-02-04  146  			/* Sleep for at least 1/5 of a second before retry. */
6b35e407 Rusty Russell      2008-02-04  147  			msleep(200);
6b35e407 Rusty Russell      2008-02-04  148  			break;
6b35e407 Rusty Russell      2008-02-04  149  		}
3ccc9372 Michael S. Tsirkin 2012-04-12  150  		set_page_pfns(vb->pfns + vb->num_pfns, page);
3ccc9372 Michael S. Tsirkin 2012-04-12  151  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
6b35e407 Rusty Russell      2008-02-04  152  		totalram_pages--;
6b35e407 Rusty Russell      2008-02-04  153  	}
6b35e407 Rusty Russell      2008-02-04  154  
6b35e407 Rusty Russell      2008-02-04  155  	/* Didn't get any?  Oh well. */
6b35e407 Rusty Russell      2008-02-04  156  	if (vb->num_pfns == 0)
6b35e407 Rusty Russell      2008-02-04 @157  		return;
6b35e407 Rusty Russell      2008-02-04  158  
6b35e407 Rusty Russell      2008-02-04  159  	tell_host(vb, vb->inflate_vq);
5f1da406 Rafael Aquini      2012-11-09  160  	mutex_unlock(&vb->balloon_lock);
Quoted 8 lines
Quoted 6 lines

This bug was fixed by

virtio_balloon-introduce-migration-primitives-to-balloon-pages.patch
this one -> virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix.patch
virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix.patch virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix-fix.patch

Michal: your since-3.6 branch somehow missed that followup fix...

Hmm strange, I can see all of them in my tree.
72d9876194be9e6f0600ca796b6689a77fce28b7
f920c4f67b892a6b41054c5441ab0d481489c6c9
63db42f4243be26efffc32806990349235619bad

merged in mmotm-2012-11-14-17-40

and my mis-merge follow-up
83bb61967444e22100f7c6e2a5f79ffa85b9e981

merged in mmotm-2012-11-26-17-32
All of them should be

Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
author
#6 lore raw
Quoted 56 lines · Michal Hocko
Quoted 55 lines · Wu Fengguang
Quoted 53 lines · Andrew Morton
Quoted 35 lines · On Fri, 30 Nov 2012 08:28:48 +0800 Fengguang Wu <[email protected]> wrote:

Hi Rafael,

[Julia and me think that this coccinelle warning is worth reporting.]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 5f1da4063294480b3fabcee554f976565dec54b5 [341/499] virtio_balloon: introduce migration primitives to balloon pages

+ drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

vim +157 drivers/virtio/virtio_balloon.c

6b35e407 Rusty Russell      2008-02-04  130  {
5f1da406 Rafael Aquini      2012-11-09  131  	struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
5f1da406 Rafael Aquini      2012-11-09  132  
6b35e407 Rusty Russell      2008-02-04  133  	/* We can only do one array worth at a time. */
6b35e407 Rusty Russell      2008-02-04  134  	num = min(num, ARRAY_SIZE(vb->pfns));
6b35e407 Rusty Russell      2008-02-04  135  
5f1da406 Rafael Aquini      2012-11-09 @136  	mutex_lock(&vb->balloon_lock);
3ccc9372 Michael S. Tsirkin 2012-04-12  137  	for (vb->num_pfns = 0; vb->num_pfns < num;
3ccc9372 Michael S. Tsirkin 2012-04-12  138  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
5f1da406 Rafael Aquini      2012-11-09  139  		struct page *page = balloon_page_enqueue(vb_dev_info);
5f1da406 Rafael Aquini      2012-11-09  140  
6b35e407 Rusty Russell      2008-02-04  141  		if (!page) {
6b35e407 Rusty Russell      2008-02-04  142  			if (printk_ratelimit())
6b35e407 Rusty Russell      2008-02-04  143  				dev_printk(KERN_INFO, &vb->vdev->dev,
6b35e407 Rusty Russell      2008-02-04  144  					   "Out of puff! Can't get %zu pages\n",
5f1da406 Rafael Aquini      2012-11-09  145  					   VIRTIO_BALLOON_PAGES_PER_PAGE);
6b35e407 Rusty Russell      2008-02-04  146  			/* Sleep for at least 1/5 of a second before retry. */
6b35e407 Rusty Russell      2008-02-04  147  			msleep(200);
6b35e407 Rusty Russell      2008-02-04  148  			break;
6b35e407 Rusty Russell      2008-02-04  149  		}
3ccc9372 Michael S. Tsirkin 2012-04-12  150  		set_page_pfns(vb->pfns + vb->num_pfns, page);
3ccc9372 Michael S. Tsirkin 2012-04-12  151  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
6b35e407 Rusty Russell      2008-02-04  152  		totalram_pages--;
Quoted 8 lines
6b35e407 Rusty Russell      2008-02-04  153  	}
6b35e407 Rusty Russell      2008-02-04  154  
6b35e407 Rusty Russell      2008-02-04  155  	/* Didn't get any?  Oh well. */
6b35e407 Rusty Russell      2008-02-04  156  	if (vb->num_pfns == 0)
6b35e407 Rusty Russell      2008-02-04 @157  		return;
6b35e407 Rusty Russell      2008-02-04  158  
6b35e407 Rusty Russell      2008-02-04  159  	tell_host(vb, vb->inflate_vq);
5f1da406 Rafael Aquini      2012-11-09  160  	mutex_unlock(&vb->balloon_lock);

This bug was fixed by

virtio_balloon-introduce-migration-primitives-to-balloon-pages.patch
this one -> virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix.patch
virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix.patch virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix-fix.patch

Quoted 6 lines
Quoted 1 line

Michal: your since-3.6 branch somehow missed that followup fix...

Hmm strange, I can see all of them in my tree.
72d9876194be9e6f0600ca796b6689a77fce28b7
f920c4f67b892a6b41054c5441ab0d481489c6c9
63db42f4243be26efffc32806990349235619bad

Oops.. the fixes are reverted by a later commit 4f2ac849

- /* Did we get any? */
- if (vb->num_pfns != 0)
- tell_host(vb, vb->inflate_vq);
- mutex_unlock(&vb->balloon_lock);
+ /* Didn't get any? Oh well. */
(*) + if (vb->num_pfns == 0)
(*) + return;
+
+ tell_host(vb, vb->inflate_vq);

(*) then we got the coccinelle warning again in the HEAD.

Thanks,
Fengguang

Quoted 11 lines

merged in mmotm-2012-11-14-17-40

and my mis-merge follow-up
83bb61967444e22100f7c6e2a5f79ffa85b9e981

merged in mmotm-2012-11-26-17-32
All of them should be

Signature · 2 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#7 lore raw
Quoted 57 lines · Wu Fengguang
Quoted 56 lines · Michal Hocko
Quoted 55 lines · Wu Fengguang
Quoted 53 lines · Andrew Morton
Quoted 35 lines · On Fri, 30 Nov 2012 08:28:48 +0800 Fengguang Wu <[email protected]> wrote:

Hi Rafael,

[Julia and me think that this coccinelle warning is worth reporting.]

tree:   git://git.kernel.org/pub/scm/linux/kernel/git/mhocko/mm.git since-3.6
head:   422a0f651b5cefa1b6b3ede2e1c9e540a24a6e01
commit: 5f1da4063294480b3fabcee554f976565dec54b5 [341/499] virtio_balloon: introduce migration primitives to balloon pages

+ drivers/virtio/virtio_balloon.c:157:2-8: preceding lock on line 136

vim +157 drivers/virtio/virtio_balloon.c

6b35e407 Rusty Russell      2008-02-04  130  {
5f1da406 Rafael Aquini      2012-11-09  131  	struct balloon_dev_info *vb_dev_info = vb->vb_dev_info;
5f1da406 Rafael Aquini      2012-11-09  132  
6b35e407 Rusty Russell      2008-02-04  133  	/* We can only do one array worth at a time. */
6b35e407 Rusty Russell      2008-02-04  134  	num = min(num, ARRAY_SIZE(vb->pfns));
6b35e407 Rusty Russell      2008-02-04  135  
5f1da406 Rafael Aquini      2012-11-09 @136  	mutex_lock(&vb->balloon_lock);
3ccc9372 Michael S. Tsirkin 2012-04-12  137  	for (vb->num_pfns = 0; vb->num_pfns < num;
3ccc9372 Michael S. Tsirkin 2012-04-12  138  	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
5f1da406 Rafael Aquini      2012-11-09  139  		struct page *page = balloon_page_enqueue(vb_dev_info);
5f1da406 Rafael Aquini      2012-11-09  140  
6b35e407 Rusty Russell      2008-02-04  141  		if (!page) {
6b35e407 Rusty Russell      2008-02-04  142  			if (printk_ratelimit())
6b35e407 Rusty Russell      2008-02-04  143  				dev_printk(KERN_INFO, &vb->vdev->dev,
6b35e407 Rusty Russell      2008-02-04  144  					   "Out of puff! Can't get %zu pages\n",
5f1da406 Rafael Aquini      2012-11-09  145  					   VIRTIO_BALLOON_PAGES_PER_PAGE);
6b35e407 Rusty Russell      2008-02-04  146  			/* Sleep for at least 1/5 of a second before retry. */
6b35e407 Rusty Russell      2008-02-04  147  			msleep(200);
6b35e407 Rusty Russell      2008-02-04  148  			break;
6b35e407 Rusty Russell      2008-02-04  149  		}
3ccc9372 Michael S. Tsirkin 2012-04-12  150  		set_page_pfns(vb->pfns + vb->num_pfns, page);
3ccc9372 Michael S. Tsirkin 2012-04-12  151  		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
6b35e407 Rusty Russell      2008-02-04  152  		totalram_pages--;
Quoted 8 lines
6b35e407 Rusty Russell      2008-02-04  153  	}
6b35e407 Rusty Russell      2008-02-04  154  
6b35e407 Rusty Russell      2008-02-04  155  	/* Didn't get any?  Oh well. */
6b35e407 Rusty Russell      2008-02-04  156  	if (vb->num_pfns == 0)
6b35e407 Rusty Russell      2008-02-04 @157  		return;
6b35e407 Rusty Russell      2008-02-04  158  
6b35e407 Rusty Russell      2008-02-04  159  	tell_host(vb, vb->inflate_vq);
5f1da406 Rafael Aquini      2012-11-09  160  	mutex_unlock(&vb->balloon_lock);

This bug was fixed by

virtio_balloon-introduce-migration-primitives-to-balloon-pages.patch
this one -> virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix.patch
virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix.patch virtio_balloon-introduce-migration-primitives-to-balloon-pages-fix-fix-fix.patch

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

Michal: your since-3.6 branch somehow missed that followup fix...

Hmm strange, I can see all of them in my tree.
72d9876194be9e6f0600ca796b6689a77fce28b7
f920c4f67b892a6b41054c5441ab0d481489c6c9
63db42f4243be26efffc32806990349235619bad

Oops.. the fixes are reverted by a later commit 4f2ac849

Ohh, I really screwed that revert (this is the second issue already). Sorry about that and especially sorry that I brought more burden on you. I will be more careful next time.

Fixed and pushed

Thanks a lot!

Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>