On Mon, May 13, 2024 at 02:53:42PM +0200, Håkon Bugge wrote:
> For most entry points to RDS, we call memalloc_noio_{save,restore} in
> a parenthetic fashion when enabled by the module parameter force_noio.
>
> We skip the calls to memalloc_noio_{save,restore} in rds_ioctl(), as
> no memory allocations are executed in this function or its callees.
>
> The reason we execute memalloc_noio_{save,restore} in rds_poll(), is
> due to the following call chain:
>
> rds_poll()
> poll_wait()
> __pollwait()
> poll_get_entry()
> __get_free_page(GFP_KERNEL)
>
> The function rds_setsockopt() allocates memory in its callee's
> rds_get_mr() and rds_get_mr_for_dest(). Hence, we need
> memalloc_noio_{save,restore} in rds_setsockopt().
>
> In rds_getsockopt(), we have rds_info_getsockopt() that allocates
> memory. Hence, we need memalloc_noio_{save,restore} in
> rds_getsockopt().
>
> All the above, in order to conditionally enable RDS to become a block I/O
> device.
>
> Signed-off-by: Håkon Bugge <haakon.bugge@xxxxxxxxxx>
Hi Håkon,
Some minor feedback from my side.
> ---
> net/rds/af_rds.c | 60 +++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
> index 8435a20968ef5..a89d192aabc0b 100644
> --- a/net/rds/af_rds.c
> +++ b/net/rds/af_rds.c
> @@ -37,10 +37,16 @@ > #include <linux/in.h>
> #include <linux/ipv6.h>
> #include <linux/poll.h>
> +#include <linux/sched/mm.h>
> #include <net/sock.h>
>
> #include "rds.h"
>
> +bool rds_force_noio;
> +EXPORT_SYMBOL(rds_force_noio);
rds_force_noio seems to be only used within this file.
I wonder if it should it be static and not EXPORTed?
Flagged by Sparse.
> +module_param_named(force_noio, rds_force_noio, bool, 0444);
> +MODULE_PARM_DESC(force_noio, "Force the use of GFP_NOIO (Y/N)");
> +
> /* this is just used for stats gathering :/ */
> static DEFINE_SPINLOCK(rds_sock_lock);
> static unsigned long rds_sock_count;
> @@ -60,6 +66,10 @@ static int rds_release(struct socket *sock)
> {
> struct sock *sk = sock->sk;
> struct rds_sock *rs;
> + unsigned int noio_flags;
Please consider using reverse xmas tree order - longest line to shortest -
for local variable declarations in Networking code.
This tool can be of assistance: https://github.com/ecree-solarflare/xmastree
> +
> + if (rds_force_noio)
> + noio_flags = memalloc_noio_save();
>
> if (!sk)
> goto out;
...
> @@ -324,6 +346,8 @@ static int rds_cancel_sent_to(struct rds_sock *rs, sockptr_t optval, int len)
>
> rds_send_drop_to(rs, &sin6);
> out:
> + if (rds_force_noio)
> + noio_flags = memalloc_noio_save();
noio_flags appears to be set but otherwise unused in this function.
Flagged by W=1 builds.
> return ret;
> }
>
...
