From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753517AbcAEFvV (ORCPT ); Tue, 5 Jan 2016 00:51:21 -0500 Received: from mail-pf0-f177.google.com ([209.85.192.177]:36515 "EHLO mail-pf0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751472AbcAEFvS (ORCPT ); Tue, 5 Jan 2016 00:51:18 -0500 Date: Tue, 5 Jan 2016 13:49:06 +0800 From: Peter Chen To: "Du, Changbin" Cc: "balbi@ti.com" , "gregkh@linuxfoundation.org" , "viro@zeniv.linux.org.uk" , "mina86@mina86.com" , "r.baldyga@samsung.com" , "rui.silva@linaro.org" , "k.opasiak@samsung.com" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] usb: f_fs: avoid race condition with ffs_epfile_io_complete Message-ID: <20160105054906.GB29244@shlinux2> References: <1451371018-14918-1-git-send-email-changbin.du@intel.com> <20160105033216.GA29244@shlinux2> <0C18FE92A7765D4EB9EE5D38D86A563A05C93F77@SHSMSX103.ccr.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A05C93F77@SHSMSX103.ccr.corp.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jan 05, 2016 at 04:09:47AM +0000, Du, Changbin wrote: > > > To avoid this, just dequeue the request first. After usb_ep_dequeue, the > > > request must be done or canceled. > > > > > > With this change, we can ensure no race condition in f_fs driver. But > > > actually I found some of the udc driver has analogical issue in its > > > dequeue implementation. For example, > > > 1) the dequeue function hold the controller's lock. > > > 2) before driver request controller to stop transfer, a request > > > completed. > > > 3) the controller trigger a interrupt, but its irq handler need wait > > > dequeue function to release the lock. > > > 4) dequeue function give back the request with negative status, and > > > release lock. > > > 5) irq handler get lock but the request has already been given back. > > > > > > > get unlock? > > > > During the interrupt handler, it should only handle the "data complete" > > interrupt on queued request; if the "data complete" interrupt occurs, but > > it belongs to nobody, it will handle noop. > > > > > > Best Regards, > > Peter Chen > > You are right, but the problem is the request->status is wrong. If the data > send out but report caller as -EINTR, it will introduce duplicate-send > issue. > Why -EINTR, the kernel-doc said it should return -ECONNRESET for active request, see include/linux/usb/gadget.h. -- Best Regards, Peter Chen