Received-SPF: pass (google.com: domain of linux-kernel+bounces-169392-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 6 May 2024 15:04:56 +0800
From: Hongren Zheng <i@zenithal.me>
To: Simon Holesch <simon@holesch.de>
Cc: Valentina Manea <valentina.manea.m@gmail.com>,
	Shuah Khan <shuah@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Shuah Khan <skhan@linuxfoundation.org>, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4] usbip: Don't submit special requests twice
Message-ID: <ZjiBGLVN6GteWvti@Sun>
References: <20231217194624.102385-1-simon@holesch.de>
 <ZjemYB6CpAx4Kx5f@Sun>
 <D11WJK47MBWS.3795S2OL21M7R@holesch.de>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <D11WJK47MBWS.3795S2OL21M7R@holesch.de>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?uBx8gjcKR1B80gn54uBhxnqCyUX7azlFpBlpZ4O1J83mDTRivn7L+ig+r4su?=
 =?us-ascii?Q?cif9C/zIzH/hCoEVz40gzzIq2LI1vEn58HnVSGkcWWgQ3y8UbH8OzP5jY2aR?=
 =?us-ascii?Q?+9FuXmC06GjLmyjvJqNPFMvIx8DRuGVcj2cB8u94l2WDKb33VKxpBPp5ML6d?=
 =?us-ascii?Q?WkRMvi219Jiqd2tIc5lgNfA4OSiYQdlwKAHStOYwQwRmCP7zoIWdPahkCBp9?=
 =?us-ascii?Q?tO+LTV5v9GXcHsadvcj/G62Q5XBTKPKyxm+pbkyf6GbZBmTfst5ZSPYHV7dp?=
 =?us-ascii?Q?V6PvF9EKYeB6GBG8F01qGyhulV2D98iVVLWz79BxTkl8QnG3fuJqjxeivFbN?=
 =?us-ascii?Q?TPWT//WvgLbat6K2lEqaOt1pYyZk0zYRfFpg7gF294MzNI/wfK/+UdEf2N5n?=
 =?us-ascii?Q?h0yM1npDdbFtPTvEleLULRv9FGN+PPvUMJN34y+Qg5FtEf54lXqRdWfbkgc6?=
 =?us-ascii?Q?2GkVYwh+BvXW2vjTwICQLaDKBhHduVSiDdTy9fp13x3xPYOJTMvXZqG2Dw6g?=
 =?us-ascii?Q?9HyHMrG2jw8SLiGMN94BDmchyASblnrh1JMSKDX7wMRdn7peesZeGoUnB0re?=
 =?us-ascii?Q?4toUIgo81ku+y1iQ71gTLXTf7vP5yWqeE+d1rwN+VedKyn5FG6B4x66wR9OL?=
 =?us-ascii?Q?YPShUauTkjhYwVDzByWyJXQNamy1VV59+rct8LyAqoL19m4mSzH65dFTXFxC?=
 =?us-ascii?Q?rUoohkfQX5T9W8xf141GN9CDdDfeHfXykSFtTlG/L1njT1OUWmCTM6jjtnQW?=
 =?us-ascii?Q?QeqDpvqaGnnIDR5GT2x/xp/wxy/g3YZx17v+arXigCYPpteSEzDK28kzf5ta?=
 =?us-ascii?Q?50/svN2tvDhkwe8bKQ5EifzAXqeoVYhrjpQISaGAeHJ/lybkEbv1sTa6sYAo?=
 =?us-ascii?Q?9rAkfG7nTc0nQhRmuMsl39pGFJWGzPEqFosDxU7xKpx8pmzb3Dy474DxVJhd?=
 =?us-ascii?Q?377Hog1xRKoBMNhv3JWk5h21+cd2ZxidgghlgOVzphNf/oTqJ97P3WVXwLk3?=
 =?us-ascii?Q?9ckF3LdoXoIAPhH03nfYWmnoHotXpZ4nh7SxrDQtry5FS8Jzj2HyQS0UIG0R?=
 =?us-ascii?Q?lqDVCxE5lARRzZXbKA2phc05oMqW0DrpuWB0FcJJNzncyJcUj10BrGO19JdR?=
 =?us-ascii?Q?B7cpmMgMrMbwmsDNn6kh2PoA3A4HpqzSMlKUP9IJquVLhc8I4aC76wsEaQfM?=
 =?us-ascii?Q?QFPLlesRC3mwvsKYYv57lzGqKrIOeMn+6RI9YSjVr5rzUO2u8OLu8K5yQSuI?=
 =?us-ascii?Q?z3P07OM0jNIwVfTSs+qFbzKXRDd/IcLOq5n4+vEvOJU+jnZOVglVnrBb8ePu?=
 =?us-ascii?Q?LWSGaJvIRpqbjjkp9+byx3B2bE7ofZ4YhgKksK4vmNaJN+dFZ3W5MZC9ilDX?=
 =?us-ascii?Q?TnVVrRTKRvhaSDP9SiuwyXD9u4b6n1/kfAhiGBCqpdKZHdM0u2daGHi6TCwz?=
 =?us-ascii?Q?FxqbZCSlvuQrri4xjPVQeG+lq/4e6B5RfvdZMyG7a/5ZR7FNiNB4bQQ+iKVi?=
 =?us-ascii?Q?1hNzWxsoXede3tYTqM1bNa3rTPWRK3PHyJrVHebEaDYSTkhqK4UZ91xsQqbH?=
 =?us-ascii?Q?vNmFRt7EfNBLrOauW1QftfSHEkCoo0xnQSShQANs?=
X-OriginatorOrg: zenithal.me
X-MS-Exchange-CrossTenant-Network-Message-Id: 53f140aa-a24b-479d-03e7-08dc6d9ad7fe
X-MS-Exchange-CrossTenant-AuthSource: TYCP286MB1393.JPNP286.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 May 2024 07:04:59.7824
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 436d481c-43b1-4418-8d7f-84c1e4887cf0
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Vl46fD6d791bNrPLzUHln5nmKdjTgYa/fAh1By2Jl7OO/g6AtRsYKzsTXknqZ+6D
X-MS-Exchange-Transport-CrossTenantHeadersStamped: OS3P286MB2490

On Sun, May 05, 2024 at 07:54:36PM +0200, Simon Holesch wrote:
> On Sun May 5, 2024 at 5:31 PM CEST, Hongren Zheng wrote:
> > On Sun, Dec 17, 2023 at 08:30:40PM +0100, Simon Holesch wrote:
> > > Skip submitting URBs, when identical requests were already sent in
> > > tweak_special_requests(). Instead call the completion handler directly
> > > to return the result of the URB.
> >
> > Reproduced the behavior and this patch fixed the bahavior
> 
> Thank you for testing.
> 
> > > @@ -468,6 +477,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
> > >  	int support_sg = 1;
> > >  	int np = 0;
> > >  	int ret, i;
> > > +	int is_tweaked;
> > >  
> > >  	if (pipe == -1)
> > >  		return;
> > > @@ -580,8 +590,7 @@ static void stub_recv_cmd_submit(struct stub_device *sdev,
> > >  		priv->urbs[i]->pipe = pipe;
> > >  		priv->urbs[i]->complete = stub_complete;
> > >  
> > > -		/* no need to submit an intercepted request, but harmless? */
> > > -		tweak_special_requests(priv->urbs[i]);
> > > +		is_tweaked = tweak_special_requests(priv->urbs[i]);
> >
> > One question though, if there are mutiple urbs and one of them is
> > SET CONFIGURATION, then all of them would not be submitted,
> > as is_tweaked is a *global* flag instead of a per-urb flag.
> >
> > Now it is assumed that when the urb is SET CONFIGURATION then
> > num_urbs is 1. I assume it just happens to be the case and I do
> > not know if it holds for all scenario.
> 
> To be honest, I didn't fully understand the num_urbs > 1 case. I assumed
> this is for drivers not supporting SG and a long URB is just broken up
> into multiple ones.

I misunderstood that code path. It is indeed multiple URBs for
one PDU when the host controller for the physical device does not support SG.
And one global is_tweaked flag is enough. I think adding a comment here
would make it easier to understand.

Reviewed-By: Hongren Zheng <i@zenithal.me>
Tested-By: Hongren Zheng <i@zenithal.me>