Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 1 Jul 2022 11:13:21 -0700
From:   Lucas De Marchi <lucas.demarchi@intel.com>
To:     Luis Chamberlain <mcgrof@kernel.org>
CC:     linux-modules <linux-modules@vger.kernel.org>,
        lkml <linux-kernel@vger.kernel.org>
Subject: Re: [ANNOUNCE] kmod 30
Message-ID: <20220701181321.zgzxv7p3zrbzkuf6@ldmartin-desk2>
References: <20220630153621.3fggpqrbyvunhwfu@ldmartin-desk2>
 <Yr4fHBR08VAVbs2E@bombadil.infradead.org>
 <20220630223323.2hko2imx62embtsj@ldmartin-desk2>
 <Yr8zlqicFErzw0dc@bombadil.infradead.org>
Content-Type: text/plain; charset="us-ascii"; format=flowed
Content-Disposition: inline
In-Reply-To: <Yr8zlqicFErzw0dc@bombadil.infradead.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?1NtfLDWXgbjjwS2d+/eETsCZDZI3tfevx5mcaiVvDDgAQij27hCJOcnKgZPj?=
 =?us-ascii?Q?kycZXAkpDTQ7aFSl8Dj8r1TzgSvHnCHmC7zfBaMbRU7UPQmoyPOKQLQ2k8bo?=
 =?us-ascii?Q?qKu1alEYpSN5vnefcRfT3ljmhEXc9jrH1K16HKjXsjgBvOuVrTmfcOeCmBd5?=
 =?us-ascii?Q?Er++iexX7aTYEAQ4raxE5UqKkARYd/kiKfMMRdMMmxgqs7o6m9lp16tgN0Qh?=
 =?us-ascii?Q?9qj+hCJ+n4yGecUZDtvxm25go3BbY7vd+G/ZislqhQ6v1XSd+R6mHxNGwwWT?=
 =?us-ascii?Q?1Oxl3o+wabxU/tvBBohH/o9OIRjT/cts8s6J4ruB6jwEWPoM92v1Lg+eQGcC?=
 =?us-ascii?Q?TTLkpHwbM04g2ZbApD2dvhVdWZ0gpG1KeNnrRHzA7X3GcTET+GKB4doLP4zJ?=
 =?us-ascii?Q?rqFxjS7wkKH9xup/Cvfp89lGotFCbhcNMugm7PkK4T+kU+IESbqEaREC2r7V?=
 =?us-ascii?Q?69O53ecUd1LvTTHqQ+XPjvai6VEY4Vk39yH5GbssNCtTsdHHklZFg+ReC8Zl?=
 =?us-ascii?Q?zD8uMKJWyZdbyVG+4L8ewa2/04Xy+Q8OiVRu0jSaAYOb6D2W6GV8DNeFyPjM?=
 =?us-ascii?Q?pSUXkPG1u9dnQehUc+a1TOGli7SnB+mQeQ3dM5MPVB05oL0cplaze3La2VFs?=
 =?us-ascii?Q?kW5JybAAF+dpZHb+aRR8KM2nkB2GpqS+zia3xoivg4FZwFYKoe0hhaPU+QOT?=
 =?us-ascii?Q?2OxCSJkRty2+Kg6CPkGKGdsYTvpwR3lAezEbcw6kc25+sPohWxf9agL5H1WT?=
 =?us-ascii?Q?fwwB8vxUZGfSbcS5JcnMgQEELIHDuCA27e41vnmSKi0gON361kIN8C+syCUR?=
 =?us-ascii?Q?3bMNnZ/3yeZUNh+3GZNCLiG5JDrudEbhx1JSPv4J3IfHczbIfwfkrgcmMZ+t?=
 =?us-ascii?Q?DgjvPxo6JhkTxGFO3/LBRgcvhuoAYetd93bSURCur+1ljP26OjzjpLxCf+re?=
 =?us-ascii?Q?F20VoccsGduRu1khAKegj9YZ1GGNyfF6m0+KMPCu/waCKmndPqb8Ii0vkHB5?=
 =?us-ascii?Q?USMc6lmCN5q9yXwplO6VsIVpjT7EfFVx4uojThHu6sQ98l7uIAg6dI8qCSAB?=
 =?us-ascii?Q?fmMhPfBtLKvx0tPjh/ySCSlICrkEOXaU2OvWM6Sb8NXn/wFIsgb5usjs6a8M?=
 =?us-ascii?Q?e8D6T59WHpr3J6sZnX69WXFRg6iA/0GHPSGwcMU40ocwGQsBs0H19vHXzcFP?=
 =?us-ascii?Q?SkkCNt+zD+jgvh3d0MeeWw5/6sKn1rZaARV4XD9xkoMBKgqPZev2Ktft9dBz?=
 =?us-ascii?Q?YDwCzK3vunAI3DkcwKlobWL3truUNV/P6iCrr5c3toUSdlDktQtHZo5jgk7w?=
 =?us-ascii?Q?vckJg7WkH8hRZFCjZAD4lSBR4B3RfBKE+qYOtNAXRW92gljNip2UFXR3DMOC?=
 =?us-ascii?Q?DWjbeEMEvThncFr+RasNeY+2p/4suB9O6lcYuIq13a7q1nkiunEfSPo4jjlo?=
 =?us-ascii?Q?lMRWmYhoUkdAb8F3quk9tCMaGEcXDXtbfEWmgpeaxbSG1TpAsaZZZaxfHsNm?=
 =?us-ascii?Q?FkPPhdMEWBHQvS2j/gg/Klr1rOuCxicapbU3KQK+COf1d1zTBV4wV4D82KDG?=
 =?us-ascii?Q?bRt4WjgR1f/8Po8CI4h0YQpJgVBF9rQqn13XE/GSPXGOTt9QmEECRNxYgDuM?=
 =?us-ascii?Q?Yw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 435003fa-a184-4710-57e7-08da5b8d62e7
X-MS-Exchange-CrossTenant-AuthSource: CY5PR11MB6139.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Jul 2022 18:13:23.5441
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: MUsS6X7ZS05jR/gyhijEpkuDHbx/BjSzKLuR0JM1IAN3E55fDVwKygbJfv0gre3BFnjRnAU/I4lrzz/gtU0tiH0lzCANF6trUKORxykdKVk=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR11MB4164
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, Jul 01, 2022 at 10:49:10AM -0700, Luis Chamberlain wrote:
>On Thu, Jun 30, 2022 at 03:33:23PM -0700, Lucas De Marchi wrote:
>> On Thu, Jun 30, 2022 at 03:09:32PM -0700, Luis Chamberlain wrote:
>> > Sorry for the super late review, I was swamped. OK so the only issue
>> > I can think of is that rmmod *used* to support the kernel wait support
>> > with $(rmmod --wait) so wouldn't this be odd?
>>
>> any reason not to use modprobe -r?
>
>I was referring to old scripts which may have used $(rmmod --wait) before.
>But since support for that was ripped, then yeah I can see that should
>not be an issue.
>
>However I can think of *one* issue, did we ever support `modprobe--wait`?

no

>
>Because the way fstests / blktests would implement this feature
>detection is with something like this now:
>
>_has_modprobe_patient()
>{
>	modprobe --help >& /dev/null || return 1
>	modprobe --help | grep -q -1 "remove-patiently" || return 1
>	return 0
>}

the grep would need to be changed to something like "-w, --wait"

>
>> > It is why I had gone with:
>> >
>> > -p | --remove-patiently   patiently removes the module
>> > -t | --timeout            timeout in ms to remove the module
>> >
>> > You would know better though.
>> >
>> > Also just curious, is it really terrible to just support waiting
>> > forever?
>>
>> is there a use case for that? If we are trying to cover some races, I
>> imagine a small timeout would be sufficient. Also notice that if the
>> timeout is too big, so will be the interval between the retries.  On
>> your v2 I had suggested polling the refcnt so we would get notificed
>> on changes, but as you also noticed, that didn't work very well.  So I
>> went back to a time-based retry solution.
>>
>> if there is a use-case, should we cap the interval between retries?
>
>I really can't think of a use case except for catching glaring
>unexpected bugs in test suites where the kernel developer would
>really like to know something really bad happened, but even then
>a timeout is likely desirable.

yeah... if developer wants to possibly wait for a long time, `--wait -1`
is available to wait for years.

>
>So just a heads up the timeout I'll use for fstests / blktests will be
>of 100 seconds.

yeah... 100 seconds still make more sense than -1 IMO


Lucas De Marchi

>
>Thanks for this work!
>
>  Luis