Re: [PATCH] cifs: bugfix for unreclaimed writeback pages in cifs_writev_requeue()

[Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Wed, 6 Feb 2013 12:00:08 -0600
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> Forwarding to the current cifs development mailing list
> 
> 
> ---------- Forwarded message ----------
> From:  <jiang.biao2-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
> Date: Wed, Feb 6, 2013 at 12:26 AM
> Subject: [PATCH] cifs: bugfix for unreclaimed writeback pages in
> cifs_writev_requeue()
> To: Steve French <sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>, linux-cifs-client-w/Ol4Ecudplf7j03Gpi2xA@public.gmane.orgrg
> 
> 
> 
>     This patch applies to the kernel version of linux-3.8-rc6.
> 
>     Pages get the PG_writeback flag set before cifs sends its request
> to SMB server
>     in cifs_writepages(), if the SMB service goes down, cifs may try
> to recommit the
>     writing requests in cifs_writev_requeue(). However, it does not
> clean its PG_writeback
>     flag and relaimed the pages even if it fails again in
> cifs_writev_requeue(), which
>     may lead to the hanging of the processes accessing the cifs
> directory. This patch
>     just cleans the PG_writeback flags and reclaims the pages under
> that circumstances.
> 
>     Steps to reproduce the bug(trying serveral times may trigger the issue):
>     1.Write from cifs client continuously.(e.g dd if=/dev/zero of=<cifs file>)
>     2.Stop SMB service from server.(e.g service smb stop)
>     3.Wait for two minutes， and then start SMB service from
> server.(e.g service smb start)
>     4.The processes which are accessing cifs directory may hang up.
> 
>     Signed-off-by: Ouyang Maochun <ouyang.maochun-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
>     Signed-off-by: Jiang Yong <jian.yong5-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
>     Tested-by: Zhang Xianwei <zhang.xianwei8-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
>     Reviewed-by: Wang Liang <wang.liang82-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
>     Reviewed-by: Cai Qu <cai.qu-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
>     Reviewed-by: Jiang Biao <jiang.biao2-Th6q7B73Y6EnDS1+zs4M5A@public.gmane.org>
> 
> diff -uprN linux-3.8-rc6/fs/cifs/cifssmb.c linux-3.8-rc6_new/fs/cifs/cifssmb.c
> --- linux-3.8-rc6/fs/cifs/cifssmb.c        2013-02-01 09:08:14.000000000 +0800
> +++ linux-3.8-rc6_new/fs/cifs/cifssmb.c        2013-02-04
> 15:44:09.869254397 +0800
> @@ -1909,8 +1909,11 @@ cifs_writev_requeue(struct cifs_writedat
>          } while (rc == -EAGAIN);
> 
>          for (i = 0; i < wdata->nr_pages; i++) {
> -                if (rc != 0)
> +                if (rc != 0){
>                          SetPageError(wdata->pages[i]);
> +                         end_page_writeback(wdata->pages[i]);
> +                         page_cache_release(wdata->pages[i]);
> +                 }
>                  unlock_page(wdata->pages[i]);
>          }
> 
> 

Patch looks correct to me. Nice catch.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>