From mboxrd@z Thu Jan  1 00:00:00 1970
From: Josef Bacik <josef@redhat.com>
Subject: Re: [PATCH] fanotify: to differ file access event from different
 threads
Date: Thu, 13 Oct 2011 09:38:15 -0400
Message-ID: <20111013133815.GC2310@localhost.localdomain>
References: <CAE8kqZKQrDK4SAKc8vK7Wi46dGQBw6_LX2NFvkr-XZr_KmGhrQ@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: linux-fsdevel@vger.kernel.org, eparis@redhat.com
To: boyd yang <boyd.yang@gmail.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:51366 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753017Ab1JMNiS (ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
	Thu, 13 Oct 2011 09:38:18 -0400
Content-Disposition: inline
In-Reply-To: <CAE8kqZKQrDK4SAKc8vK7Wi46dGQBw6_LX2NFvkr-XZr_KmGhrQ@mail.gmail.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Thu, Oct 13, 2011 at 04:56:43PM +0800, boyd yang wrote:
> This patch fixes a hang problem of Eric Paris's fs Notification/fanotify.
> 
> Fanotify brings a way to intercept file access events.
> When multiple threadsiterate the same direcotry, some thread will hang.
> This patch let fanotify to differ access events from different
> threads, prevent fanotify from merging access events from different
> threads.
> 

You need to run this through checkpatch.pl, you have a ton of formatting
problems.  Also your email client seems to have word-wrapped parts of this, so
use a email client that doesn't word wrap.

> =============================================================
> 
> diff -r -u linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c
> linux-3.1-rc4/fs/notify/fanotify/fanotify.c
> --- linux-3.1-rc4_orig/fs/notify/fanotify/fanotify.c	2011-08-29
> 12:16:01.000000000 +0800
> +++ linux-3.1-rc4/fs/notify/fanotify/fanotify.c	2011-10-10
> 12:28:23.276847000 +0800
> @@ -15,7 +15,8 @@
> 
>  	if (old->to_tell == new->to_tell &&
>  	    old->data_type == new->data_type &&
> -	    old->tgid == new->tgid) {
> +	    old->tgid == new->tgid &&
> +	    old->pid == new->pid) {
>  		switch (old->data_type) {
>  		case (FSNOTIFY_EVENT_PATH):
>  			if ((old->path.mnt == new->path.mnt) &&
> @@ -144,11 +145,19 @@
>  		return PTR_ERR(notify_event);
> 
>  #ifdef CONFIG_FANOTIFY_ACCESS_PERMISSIONS
> -	if (event->mask & FAN_ALL_PERM_EVENTS) {
> -		/* if we merged we need to wait on the new event */
> -		if (notify_event)
> -			event = notify_event;
> -		ret = fanotify_get_response_from_access(group, event);
> +	//if overflow, do not wait for response
> +	if(fsnotify_isoverflow(event))
> +	{
> +		pr_debug("fanotify overflow!\n");
> +	}
> +	else
> +	{
> +		if (event->mask & FAN_ALL_PERM_EVENTS) {
> +			/* if we merged we need to wait on the new event */
> +			if (notify_event)
> +				event = notify_event;
> +			ret = fanotify_get_response_from_access(group, event);
> +		}
>  	}

The overflow event should only have FS_Q_OVERFLOW set in it's mask right?  So
why is this test needed at all?  Thanks,

Josef