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:   Thu, 30 Jun 2022 15:33:23 -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: <20220630223323.2hko2imx62embtsj@ldmartin-desk2>
References: <20220630153621.3fggpqrbyvunhwfu@ldmartin-desk2>
 <Yr4fHBR08VAVbs2E@bombadil.infradead.org>
Content-Type: text/plain; charset="us-ascii"; format=flowed
Content-Disposition: inline
In-Reply-To: <Yr4fHBR08VAVbs2E@bombadil.infradead.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?zasfGHyEZ/W+HIliN1JQHRtCOyna3ShXrmbDK3QVbL/SIY7PqbCwXSYwi/jC?=
 =?us-ascii?Q?XzcIg89g/D3yoxNhiBBTkDmhfeZLRylDnraP8Y7TZZMIFa5mLqa5XXIWus8g?=
 =?us-ascii?Q?NjJVbkGp9IrD/zTZUHOiN/L4sFr7P6THXXoqjexlwJW5issuo4lrTHypZ3bu?=
 =?us-ascii?Q?boJdnC5PlDMQ/LuNigqG+vXJ3vxljfntuWSdwnC5bJ7WIRkSzVCkm1V7MGf2?=
 =?us-ascii?Q?ros1rIEp3nIw59KJU/MJQP5T9VQqf4BgTlc7s2qM40hEM5ErSjRSuuPvfX1E?=
 =?us-ascii?Q?6n60hxik8y0sRHHz3NdWKcu2PHkREnojQ6hhOTV+32L8vA1YruLzRyIWFHDJ?=
 =?us-ascii?Q?2xzEBUZ0jbxhA+N1XeewhZwbMwWgowa1nzrG0IiCHuG0seIK6vQAhkhEymwB?=
 =?us-ascii?Q?QscSV0FBuAp3lMQq1hdDPxW6th5X2lcvWCDsphj/aRbzpTQqFSGzyQaN5age?=
 =?us-ascii?Q?+J0Fkma2VN3op12Upr9iaeT4us7caBbnecv3AmkeQIv/MJVBEKuSqXw5xnxo?=
 =?us-ascii?Q?6ZXQQu/BJX0SKHFS5cXH9MvnK96CKFYo92tgX8/xLbt0Qt9nKR4nqwiEcOF2?=
 =?us-ascii?Q?EhL7a8621KXuXMakOyTJNOuRRX/7CVnD+9bzHLW9DG7YElxqCDQPPqkrH9Kx?=
 =?us-ascii?Q?gfiH2UsHKSDNW4jBLzwd3YlJbFpqEPkEN6OUx+FFGPxErYo2PUUQqJ0/+Nni?=
 =?us-ascii?Q?ug1As23Ok1tp7gwNpQTucPkr1BeIZ7dq9u2JtadmTQBD/frYf8LBb8l9HPG2?=
 =?us-ascii?Q?59SdU29gpT2BLAWBTGtfwoOa3wPm3bdQ8q+qrzVsrhilG3utF9cb+9meaBvv?=
 =?us-ascii?Q?RZex9QkJZMthii/zdgdusuo72IYqPNKwx2p9jnrhzE4m4rZFQLmcdpokZJMD?=
 =?us-ascii?Q?+G0XbPaOlOdBIKlhgdF2wrmiaAdApn6XK1Z/D0yljxXhCENDU73djK5XhttH?=
 =?us-ascii?Q?5hrJVaWLcHcXK+kRp7H/Y3qvJ1XdNhNRwL4VT57XP3/u0pMNXPiwWSmecfB/?=
 =?us-ascii?Q?EMJIX7kY9xorcwq+annyo0OMc5QAwGDUpIe5nEESiIZcwCGdFMlWZLmqiuVG?=
 =?us-ascii?Q?ZdfQBQTj7SdI+GDUoIkIBKwHac+OraVw6ymu0L7dfUmouPLIgkNMJ6L32yPZ?=
 =?us-ascii?Q?ew51Jamw/hYm7WRlh5RE1XCT2DP0Y5d6wh1Lb/swYO0q7udp5B/pUNhbIKUg?=
 =?us-ascii?Q?WBMY/YY97PKCmMHfECTy7qFs9iyefXeDq/WNCSqFn0vMvaZpQX7gpdMVy8Eq?=
 =?us-ascii?Q?wecfAVnUrlgskV/bzMa6LVzKeP+LksTCHqNUBkdj8hJjrpSIBM06gGammJiA?=
 =?us-ascii?Q?tBMoeS34CP1WCdYTmXnw18v2Ao7/oOfFNu/pe2WWQF9xFBipGlyAzPesC8Cy?=
 =?us-ascii?Q?IoReRmyT+yuA4YZ+us09gATafW7SjnNkNiks6X2b8wEr0MVcl3mwVU0vfRti?=
 =?us-ascii?Q?LceH0HsoAnbTFrrtgXGIIUH0S46X0OvGvlSv8+h6oPZY5BCRn3NUfzM35/yj?=
 =?us-ascii?Q?C50AukrPMwbZzAMOeTDTWGQPIC75r7II5PhRV7Sie7hsPWpVfRpXxJOWOGDt?=
 =?us-ascii?Q?OUGDtpxsDtP1TpfIJzpSVeWyA6m/jba7nrTNFfHxz6QvBYmJA1NAaJETLCeQ?=
 =?us-ascii?Q?Dg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ea47546b-14ca-4d06-5949-08da5ae88bb3
X-MS-Exchange-CrossTenant-AuthSource: CY5PR11MB6139.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jun 2022 22:33:25.0295
 (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: tcW2heKFfxV3VFAXLsdVpytmF+cELq+c9/yPb+SgZgsKevxISboZxb9gR+jY8dYZqKoCgppcLN896KlQwOVVFXBponzs4SITCaW5HYzhoIQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR11MB5270
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        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 Thu, Jun 30, 2022 at 03:09:32PM -0700, Luis Chamberlain wrote:
>On Thu, Jun 30, 2022 at 08:36:21AM -0700, Lucas De Marchi wrote:
>>         - modprobe learned a --wait <MSEC> option to be used together with -r
>>           when removing a module. This allows modprobe to keep trying the
>>           removal if it fails because the module is still in use. An exponential backoff
>>           time is used for further retries.
>>
>>           The wait behavior provided by the kernel when not passing O_NONBLOCK
>>           to delete_module() was removed in v3.13 due to not be used and the
>>           consequences of having to support it in the kernel. However there may
>>           be some users, particularly on testsuites for individual susbsystems, that
>>           would want that. So provide a userspace implementation inside modprobe for
>>           such users. "rmmod" doesn't have a --wait as it remains a bare minimal over
>>           the API provided by the kernel. In future the --wait behavior can be added
>>           to libkmod for testsuites not exec'ing modprobe for module removal.
>
>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? Argument for rmmod supporting it in
the past is that the wait was implemented on the kernel side and rmmod
is the minimum wrapper around what the kernel provides.

On the other side, user shouldn't need to know where that is
implemented.

Over time libkmod grew much more to support loading/querying modules
rather than removing. I think for next version I will move some of the
module-removal support to libkmod rather than modprobe/rmmod. Then we
can think again on supporting that flag there.

>
>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?

thanks
Lucas De Marchi

>
>  Luis