From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AA93A2F5E; Thu, 21 Aug 2025 12:32:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755779570; cv=none; b=FaKFQCBPugfdqh4U68CwxKBVMVApsdkh9CREEDxdm1XMWU8/lFBB6gmBHAscwYGIqXaRfn6+NW4Na52faffgKzYbZekak644uLrjIg8Qe0JtHPN1JAWaMDC7upR6opBwoHy3saesEU+4eS/kiU5YxFHn0gEumEhgXYZm8haQ924= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1755779570; c=relaxed/simple; bh=SrTqUvmK+NpBlB3NybcMH+08tW6CxEGC7b7hfNmTs+o=; h=Mime-Version:Content-Type:Date:Message-Id:Subject:Cc:To:From: References:In-Reply-To; b=qaD7D2GP3USc/1HFnE4wr6yhZr31qV3io1hKVwb5FMCXA7f0nanFyZPF2coBW9mMmZFtBI873RQx89CSmLo9DEkrU6kYzuJNDOHZOI+Gcuvc/287WbNwaHmk5HqVJkrNW2D/flmJbqK/zcYPcC2c68SqaKFdqDZQy9b9qIScx8A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=uCQggHcA; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="uCQggHcA" Received: by smtp.kernel.org (Postfix) with ESMTPSA id ADD5AC4CEEB; Thu, 21 Aug 2025 12:32:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1755779570; bh=SrTqUvmK+NpBlB3NybcMH+08tW6CxEGC7b7hfNmTs+o=; h=Date:Subject:Cc:To:From:References:In-Reply-To:From; b=uCQggHcA5Li5qLEFUvSJkmHidXQNxNdMzvaTqeAw29JRR9pkLETXDXfe08yyby1cU jVDiELhFQAVMpMSslcmgWwO8sHb6M0J9GqGT29uI0UB2ILITqHU5Hr9u/n+2xxzUZt RsqrBROsXWPY03PYjVcFjbBF/otcl0NvdUtvnJZnDTuZMNmFcIpYRuFzhMkcwuziQJ tuNs4MMlFfsLQVZG7dhWb0lRWYDNt6pEwhmL299fAMqRKPu8oj7zrcJ9ZX+mrAUmzQ TT7MYIYJK6aMRtQap+bRB7fBadnrIesb6mDbR84FskI7npl1Rswn+1gv8kLsChp6t/ hIPmY9joAf4TQ== Precedence: bulk X-Mailing-List: rust-for-linux@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 21 Aug 2025 14:32:44 +0200 Message-Id: Subject: Re: [PATCH v2] rust: zpool: add abstraction for zpool drivers Cc: , , "Uladzislau Rezki" , "Alice Ryhl" , "Vlastimil Babka" , "Lorenzo Stoakes" , "Liam R . Howlett" , "Miguel Ojeda" , "Alex Gaynor" , "Boqun Feng" , "Gary Guo" , "Bjorn Roy Baron" , "Benno Lossin" , "Andreas Hindborg" , "Trevor Gross" , "Johannes Weiner" , "Yosry Ahmed" , "Nhat Pham" , To: "Vitaly Wool" From: "Danilo Krummrich" References: <20250821111718.512936-1-vitaly.wool@konsulko.se> In-Reply-To: On Thu Aug 21, 2025 at 2:03 PM CEST, Danilo Krummrich wrote: > On Thu Aug 21, 2025 at 1:17 PM CEST, Vitaly Wool wrote: >> + /// preferred NUMA node `nid`. If the allocation is successful, an = opaque handle is returned. >> + fn malloc( >> + pool: ::BorrowedMut<'_>, >> + size: usize, >> + gfp: Flags, >> + nid: NumaNode, >> + ) -> Result; > > I still think we need a proper type representation of a zpool handle that > guarantees validity and manages its lifetime. > > For instance, what prevents a caller from calling write() with a random h= andle? > > Looking at zsmalloc(), if I call write() with a random number, I will mos= t > likely oops the kernel. This is not acceptable for safe APIs. > > Alternatively, all those trait functions have to be unsafe, which would b= e very > unfortunate. I just noticed that I confused something here. :) So, for the backend driver this trait is obviously fine, since you have to = implement the C ops -- sorry for the confusion. However, you still have to mark all functions except alloc() and total_page= s() as unsafe and document and justify the corresponding safety requirements. >> + /// Free a previously allocated from the `pool` object, represented= by `handle`. >> + fn free(pool: ::Borrowed<'_>, handle:= usize); > > What happens if I forget to call free()? > >> + /// Make all the necessary preparations for the caller to be able t= o read from the object >> + /// represented by `handle` and return a valid pointer to the `hand= le` memory to be read. >> + fn read_begin(pool: ::Borrowed<'_>, h= andle: usize) >> + -> NonNull; > > Same for this, making it a NonNull is better than a *mut c_void, but = it's > still a raw pointer. Nothing prevents users from using this raw pointer a= fter > read_end() has been called. > > This needs a type representation that only lives until read_end(). > > In general, I think this design doesn't really work out well. I think the= design > should be something along the lines of: > > (1) We should only provide alloc() on the Zpool itself and which return= s a > Zmem instance. A Zmem instance must not outlive the Zpool it was al= located > with. > > (2) Zmem should call free() when it is dropped. It should provide read_= begin() > and write() methods. > > (3) Zmem::read_begin() should return a Zslice which must not outlive Zm= em and > calls read_end() when dropped. This design is obiously for when you want to use a Zpool, but not implement= its backend. :)