public inbox for [email protected]
 help / color / mirror / Atom feed
* Re: [PATCH v2 net-next 2/4] af_unix: Return struct unix_sock from unix_get_socket().
       [not found] ` <[email protected]>
@ 2023-11-27  9:08   ` Paolo Abeni
  2023-11-27 12:33     ` Pavel Begunkov
  0 siblings, 1 reply; 2+ messages in thread
From: Paolo Abeni @ 2023-11-27  9:08 UTC (permalink / raw)
  To: Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski
  Cc: Ivan Babrou, Kuniyuki Iwashima, netdev, Jens Axboe,
	Pavel Begunkov, io-uring

On Wed, 2023-11-22 at 17:47 -0800, Kuniyuki Iwashima wrote:
> Currently, unix_get_socket() returns struct sock, but after calling
> it, we always cast it to unix_sk().
> 
> Let's return struct unix_sock from unix_get_socket().
> 
> Signed-off-by: Kuniyuki Iwashima <[email protected]>
> ---
>  include/linux/io_uring.h |  4 ++--
>  include/net/af_unix.h    |  2 +-
>  io_uring/io_uring.c      |  5 +++--
>  net/unix/garbage.c       | 19 +++++++------------
>  net/unix/scm.c           | 26 +++++++++++---------------
>  5 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
> index aefb73eeeebf..be16677f0e4c 100644
> --- a/include/linux/io_uring.h
> +++ b/include/linux/io_uring.h
> @@ -54,7 +54,7 @@ int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
>  			      struct iov_iter *iter, void *ioucmd);
>  void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret, ssize_t res2,
>  			unsigned issue_flags);
> -struct sock *io_uring_get_socket(struct file *file);
> +struct unix_sock *io_uring_get_socket(struct file *file);
>  void __io_uring_cancel(bool cancel_all);
>  void __io_uring_free(struct task_struct *tsk);
>  void io_uring_unreg_ringfd(void);
> @@ -111,7 +111,7 @@ static inline void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
>  			void (*task_work_cb)(struct io_uring_cmd *, unsigned))
>  {
>  }
> -static inline struct sock *io_uring_get_socket(struct file *file)
> +static inline struct unix_sock *io_uring_get_socket(struct file *file)
>  {
>  	return NULL;
>  }
> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
> index 5a8a670b1920..c628d30ceb19 100644
> --- a/include/net/af_unix.h
> +++ b/include/net/af_unix.h
> @@ -14,7 +14,7 @@ void unix_destruct_scm(struct sk_buff *skb);
>  void io_uring_destruct_scm(struct sk_buff *skb);
>  void unix_gc(void);
>  void wait_for_unix_gc(void);
> -struct sock *unix_get_socket(struct file *filp);
> +struct unix_sock *unix_get_socket(struct file *filp);
>  struct sock *unix_peer_get(struct sock *sk);
>  
>  #define UNIX_HASH_MOD	(256 - 1)
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index ed254076c723..daed897f5975 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -177,13 +177,14 @@ static struct ctl_table kernel_io_uring_disabled_table[] = {
>  };
>  #endif
>  
> -struct sock *io_uring_get_socket(struct file *file)
> +struct unix_sock *io_uring_get_socket(struct file *file)
>  {
>  #if defined(CONFIG_UNIX)
>  	if (io_is_uring_fops(file)) {
>  		struct io_ring_ctx *ctx = file->private_data;
>  
> -		return ctx->ring_sock->sk;
> +		if (ctx->ring_sock->sk)
> +			return unix_sk(ctx->ring_sock->sk);
>  	}
>  #endif
>  	return NULL;
> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
> index db1bb99bb793..4d634f5f6a55 100644
> --- a/net/unix/garbage.c
> +++ b/net/unix/garbage.c
> @@ -105,20 +105,15 @@ static void scan_inflight(struct sock *x, void (*func)(struct unix_sock *),
>  
>  			while (nfd--) {
>  				/* Get the socket the fd matches if it indeed does so */
> -				struct sock *sk = unix_get_socket(*fp++);
> +				struct unix_sock *u = unix_get_socket(*fp++);
>  
> -				if (sk) {
> -					struct unix_sock *u = unix_sk(sk);
> +				/* Ignore non-candidates, they could have been added
> +				 * to the queues after starting the garbage collection
> +				 */
> +				if (u && test_bit(UNIX_GC_CANDIDATE, &u->gc_flags)) {
> +					hit = true;
>  
> -					/* Ignore non-candidates, they could
> -					 * have been added to the queues after
> -					 * starting the garbage collection
> -					 */
> -					if (test_bit(UNIX_GC_CANDIDATE, &u->gc_flags)) {
> -						hit = true;
> -
> -						func(u);
> -					}
> +					func(u);
>  				}
>  			}
>  			if (hit && hitlist != NULL) {
> diff --git a/net/unix/scm.c b/net/unix/scm.c
> index 4b3979272a81..36ce8fed9acc 100644
> --- a/net/unix/scm.c
> +++ b/net/unix/scm.c
> @@ -21,9 +21,8 @@ EXPORT_SYMBOL(gc_inflight_list);
>  DEFINE_SPINLOCK(unix_gc_lock);
>  EXPORT_SYMBOL(unix_gc_lock);
>  
> -struct sock *unix_get_socket(struct file *filp)
> +struct unix_sock *unix_get_socket(struct file *filp)
>  {
> -	struct sock *u_sock = NULL;
>  	struct inode *inode = file_inode(filp);
>  
>  	/* Socket ? */
> @@ -34,12 +33,13 @@ struct sock *unix_get_socket(struct file *filp)
>  
>  		/* PF_UNIX ? */
>  		if (s && ops && ops->family == PF_UNIX)
> -			u_sock = s;
> -	} else {
> -		/* Could be an io_uring instance */
> -		u_sock = io_uring_get_socket(filp);
> +			return unix_sk(s);
> +
> +		return NULL;
>  	}
> -	return u_sock;
> +
> +	/* Could be an io_uring instance */
> +	return io_uring_get_socket(filp);
>  }
>  EXPORT_SYMBOL(unix_get_socket);
>  
> @@ -48,13 +48,11 @@ EXPORT_SYMBOL(unix_get_socket);
>   */
>  void unix_inflight(struct user_struct *user, struct file *fp)
>  {
> -	struct sock *s = unix_get_socket(fp);
> +	struct unix_sock *u = unix_get_socket(fp);
>  
>  	spin_lock(&unix_gc_lock);
>  
> -	if (s) {
> -		struct unix_sock *u = unix_sk(s);
> -
> +	if (u) {
>  		if (!u->inflight) {
>  			BUG_ON(!list_empty(&u->link));
>  			list_add_tail(&u->link, &gc_inflight_list);
> @@ -71,13 +69,11 @@ void unix_inflight(struct user_struct *user, struct file *fp)
>  
>  void unix_notinflight(struct user_struct *user, struct file *fp)
>  {
> -	struct sock *s = unix_get_socket(fp);
> +	struct unix_sock *u = unix_get_socket(fp);
>  
>  	spin_lock(&unix_gc_lock);
>  
> -	if (s) {
> -		struct unix_sock *u = unix_sk(s);
> -
> +	if (u) {
>  		BUG_ON(!u->inflight);
>  		BUG_ON(list_empty(&u->link));
>  

Adding the io_uring peoples to the recipient list for awareness. I
guess this deserves an explicit ack from them.

Cheers,

Paolo



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2 net-next 2/4] af_unix: Return struct unix_sock from unix_get_socket().
  2023-11-27  9:08   ` [PATCH v2 net-next 2/4] af_unix: Return struct unix_sock from unix_get_socket() Paolo Abeni
@ 2023-11-27 12:33     ` Pavel Begunkov
  0 siblings, 0 replies; 2+ messages in thread
From: Pavel Begunkov @ 2023-11-27 12:33 UTC (permalink / raw)
  To: Paolo Abeni, Kuniyuki Iwashima, David S. Miller, Eric Dumazet,
	Jakub Kicinski
  Cc: Ivan Babrou, Kuniyuki Iwashima, netdev, Jens Axboe, io-uring

On 11/27/23 09:08, Paolo Abeni wrote:
> On Wed, 2023-11-22 at 17:47 -0800, Kuniyuki Iwashima wrote:
>> Currently, unix_get_socket() returns struct sock, but after calling
>> it, we always cast it to unix_sk().
>>
>> Let's return struct unix_sock from unix_get_socket().
>>
>> Signed-off-by: Kuniyuki Iwashima <[email protected]>
>> ---
>>   include/linux/io_uring.h |  4 ++--
>>   include/net/af_unix.h    |  2 +-
>>   io_uring/io_uring.c      |  5 +++--
>>   net/unix/garbage.c       | 19 +++++++------------
>>   net/unix/scm.c           | 26 +++++++++++---------------
>>   5 files changed, 24 insertions(+), 32 deletions(-)
>>
>> diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h
>> index aefb73eeeebf..be16677f0e4c 100644
>> --- a/include/linux/io_uring.h
>> +++ b/include/linux/io_uring.h
>> @@ -54,7 +54,7 @@ int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
>>   			      struct iov_iter *iter, void *ioucmd);
>>   void io_uring_cmd_done(struct io_uring_cmd *cmd, ssize_t ret, ssize_t res2,
>>   			unsigned issue_flags);
>> -struct sock *io_uring_get_socket(struct file *file);
>> +struct unix_sock *io_uring_get_socket(struct file *file);
>>   void __io_uring_cancel(bool cancel_all);
>>   void __io_uring_free(struct task_struct *tsk);
>>   void io_uring_unreg_ringfd(void);
>> @@ -111,7 +111,7 @@ static inline void io_uring_cmd_do_in_task_lazy(struct io_uring_cmd *ioucmd,
>>   			void (*task_work_cb)(struct io_uring_cmd *, unsigned))
>>   {
>>   }
>> -static inline struct sock *io_uring_get_socket(struct file *file)
>> +static inline struct unix_sock *io_uring_get_socket(struct file *file)
>>   {
>>   	return NULL;
>>   }
>> diff --git a/include/net/af_unix.h b/include/net/af_unix.h
>> index 5a8a670b1920..c628d30ceb19 100644
>> --- a/include/net/af_unix.h
>> +++ b/include/net/af_unix.h
>> @@ -14,7 +14,7 @@ void unix_destruct_scm(struct sk_buff *skb);
>>   void io_uring_destruct_scm(struct sk_buff *skb);
>>   void unix_gc(void);
>>   void wait_for_unix_gc(void);
>> -struct sock *unix_get_socket(struct file *filp);
>> +struct unix_sock *unix_get_socket(struct file *filp);
>>   struct sock *unix_peer_get(struct sock *sk);
>>   
>>   #define UNIX_HASH_MOD	(256 - 1)
>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>> index ed254076c723..daed897f5975 100644
>> --- a/io_uring/io_uring.c
>> +++ b/io_uring/io_uring.c
>> @@ -177,13 +177,14 @@ static struct ctl_table kernel_io_uring_disabled_table[] = {
>>   };
>>   #endif
>>   
>> -struct sock *io_uring_get_socket(struct file *file)
>> +struct unix_sock *io_uring_get_socket(struct file *file)
>>   {
>>   #if defined(CONFIG_UNIX)
>>   	if (io_is_uring_fops(file)) {
>>   		struct io_ring_ctx *ctx = file->private_data;
>>   
>> -		return ctx->ring_sock->sk;
>> +		if (ctx->ring_sock->sk)
>> +			return unix_sk(ctx->ring_sock->sk);
>>   	}
>>   #endif
>>   	return NULL;
>> diff --git a/net/unix/garbage.c b/net/unix/garbage.c
>> index db1bb99bb793..4d634f5f6a55 100644
>> --- a/net/unix/garbage.c
>> +++ b/net/unix/garbage.c
>> @@ -105,20 +105,15 @@ static void scan_inflight(struct sock *x, void (*func)(struct unix_sock *),
>>   
>>   			while (nfd--) {
>>   				/* Get the socket the fd matches if it indeed does so */
>> -				struct sock *sk = unix_get_socket(*fp++);
>> +				struct unix_sock *u = unix_get_socket(*fp++);
>>   
>> -				if (sk) {
>> -					struct unix_sock *u = unix_sk(sk);
>> +				/* Ignore non-candidates, they could have been added
>> +				 * to the queues after starting the garbage collection
>> +				 */
>> +				if (u && test_bit(UNIX_GC_CANDIDATE, &u->gc_flags)) {
>> +					hit = true;
>>   
>> -					/* Ignore non-candidates, they could
>> -					 * have been added to the queues after
>> -					 * starting the garbage collection
>> -					 */
>> -					if (test_bit(UNIX_GC_CANDIDATE, &u->gc_flags)) {
>> -						hit = true;
>> -
>> -						func(u);
>> -					}
>> +					func(u);
>>   				}
>>   			}
>>   			if (hit && hitlist != NULL) {
>> diff --git a/net/unix/scm.c b/net/unix/scm.c
>> index 4b3979272a81..36ce8fed9acc 100644
>> --- a/net/unix/scm.c
>> +++ b/net/unix/scm.c
>> @@ -21,9 +21,8 @@ EXPORT_SYMBOL(gc_inflight_list);
>>   DEFINE_SPINLOCK(unix_gc_lock);
>>   EXPORT_SYMBOL(unix_gc_lock);
>>   
>> -struct sock *unix_get_socket(struct file *filp)
>> +struct unix_sock *unix_get_socket(struct file *filp)
>>   {
>> -	struct sock *u_sock = NULL;
>>   	struct inode *inode = file_inode(filp);
>>   
>>   	/* Socket ? */
>> @@ -34,12 +33,13 @@ struct sock *unix_get_socket(struct file *filp)
>>   
>>   		/* PF_UNIX ? */
>>   		if (s && ops && ops->family == PF_UNIX)
>> -			u_sock = s;
>> -	} else {
>> -		/* Could be an io_uring instance */
>> -		u_sock = io_uring_get_socket(filp);
>> +			return unix_sk(s);
>> +
>> +		return NULL;
>>   	}
>> -	return u_sock;
>> +
>> +	/* Could be an io_uring instance */
>> +	return io_uring_get_socket(filp);
>>   }
>>   EXPORT_SYMBOL(unix_get_socket);
>>   
>> @@ -48,13 +48,11 @@ EXPORT_SYMBOL(unix_get_socket);
>>    */
>>   void unix_inflight(struct user_struct *user, struct file *fp)
>>   {
>> -	struct sock *s = unix_get_socket(fp);
>> +	struct unix_sock *u = unix_get_socket(fp);
>>   
>>   	spin_lock(&unix_gc_lock);
>>   
>> -	if (s) {
>> -		struct unix_sock *u = unix_sk(s);
>> -
>> +	if (u) {
>>   		if (!u->inflight) {
>>   			BUG_ON(!list_empty(&u->link));
>>   			list_add_tail(&u->link, &gc_inflight_list);
>> @@ -71,13 +69,11 @@ void unix_inflight(struct user_struct *user, struct file *fp)
>>   
>>   void unix_notinflight(struct user_struct *user, struct file *fp)
>>   {
>> -	struct sock *s = unix_get_socket(fp);
>> +	struct unix_sock *u = unix_get_socket(fp);
>>   
>>   	spin_lock(&unix_gc_lock);
>>   
>> -	if (s) {
>> -		struct unix_sock *u = unix_sk(s);
>> -
>> +	if (u) {
>>   		BUG_ON(!u->inflight);
>>   		BUG_ON(list_empty(&u->link));
>>   
> 
> Adding the io_uring peoples to the recipient list for awareness. I
> guess this deserves an explicit ack from them.

Thanks Paolo, lgtm

Acked-by: Pavel Begunkov <[email protected]>

-- 
Pavel Begunkov

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-11-27 12:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <[email protected]>
     [not found] ` <[email protected]>
2023-11-27  9:08   ` [PATCH v2 net-next 2/4] af_unix: Return struct unix_sock from unix_get_socket() Paolo Abeni
2023-11-27 12:33     ` Pavel Begunkov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox