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:   Sat, 6 Aug 2022 13:24:59 +0300
From:   Konstantin Shelekhin <k.shelekhin@yadro.com>
To:     <ojeda@kernel.org>
CC:     <alex.gaynor@gmail.com>, <ark.email@gmail.com>,
        <bjorn3_gh@protonmail.com>, <bobo1239@web.de>,
        <bonifaido@gmail.com>, <boqun.feng@gmail.com>,
        <davidgow@google.com>, <dev@niklasmohrin.de>,
        <dsosnowski@dsosnowski.pl>, <foxhlchen@gmail.com>,
        <gary@garyguo.net>, <geofft@ldpreload.com>,
        <gregkh@linuxfoundation.org>, <jarkko@kernel.org>,
        <john.m.baublitz@gmail.com>, <leseulartichaut@gmail.com>,
        <linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <m.falkowski@samsung.com>, <me@kloenk.de>, <milan@mdaverde.com>,
        <mjmouse9999@gmail.com>, <patches@lists.linux.dev>,
        <rust-for-linux@vger.kernel.org>, <thesven73@gmail.com>,
        <torvalds@linux-foundation.org>, <viktor@v-gar.de>,
        <wedsonaf@google.com>
Subject: Re: [PATCH v9 12/27] rust: add `kernel` crate
Message-ID: <Yu5Bex9zU6KJpcEm@yadro.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20220805154231.31257-13-ojeda@kernel.org>
Precedence: bulk

> +unsafe impl GlobalAlloc for KernelAllocator {
> +    unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
> +        // `krealloc()` is used instead of `kmalloc()` because the latter is
> +        // an inline function and cannot be bound to as a result.
> +        unsafe { bindings::krealloc(ptr::null(), layout.size(), bindings::GFP_KERNEL) as *mut u8 }
> +    }
> +
> +    unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) {
> +        unsafe {
> +            bindings::kfree(ptr as *const core::ffi::c_void);
> +        }
> +    }
> +}

I sense possible problems here. It's common for a kernel code to pass
flags during memory allocations.

For example:

  struct bio *bio;

  for (...) {
        bio = bio_alloc_bioset(bdev, nr_vecs, opf, GFP_NOIO, bs);
        if (!bio)
        	return -ENOMEM;
  }

Without GFP_NOIO we can run into a deadlock, because the kernel will try
give us free memory by flushing the dirty pages and we need the memory
to actually do it and boom, deadlock.

Or we can be allocating some structs under spinlock (yeah, that happens too):

  struct efc_vport *vport;

  spin_lock_irqsave(...);
  vport = kzalloc(sizeof(*vport), GFP_ATOMIC);
  if (!vport) {
  	spin_unlock_irqrestore(...);
  	return NULL;
  }
  spin_unlock(...);

Same can (and probably will) happen to e.g. Vec elements. So some form
of flags passing should be supported in try_* variants:

  let mut vec = Vec::try_new(GFP_ATOMIC)?;

  vec.try_push(GFP_ATOMIC, 1)?;
  vec.try_push(GFP_ATOMIC, 2)?;
  vec.try_push(GFP_ATOMIC, 3)?;