Re: [PATCH v3 51/55] smc: Drop smc_sendpage() in favour of smc_sendmsg() + MSG_SPLICE_PAGES

["D. Wythe" <alibuda@xxxxxxxxxxxxxxxxx>]

On 4/26/23 9:07 PM, D. Wythe wrote:
> Hi David,
>
> Fallback is one of the most important features of SMC, which 
> automatically downgrades to TCP
> when SMC discovers that the peer does not support SMC. After fallback, 
> SMC hopes the the ability can be
> consistent with that of TCP sock. If you delete the smc_sendpage, when 
> fallback occurs, it means that the sock after the fallback
> loses the ability of  sendpage( tcp_sendpage).
>
> Thanks
> D. Wythe

Sorry, I missed the key email context. The problem mentioned here does 
not exist ...

> On 4/1/23 12:09 AM, David Howells wrote:
> > Drop the smc_sendpage() code as smc_sendmsg() just passes the call 
> > down to
> > the underlying TCP socket and smc_tx_sendpage() is just a wrapper around
> > its sendmsg implementation.
> > Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> > cc: Karsten Graul <kgraul@xxxxxxxxxxxxx>
> > cc: Wenjia Zhang <wenjia@xxxxxxxxxxxxx>
> > cc: Jan Karcher <jaka@xxxxxxxxxxxxx>
> > cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
> > cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> > cc: Jakub Kicinski <kuba@xxxxxxxxxx>
> > cc: Paolo Abeni <pabeni@xxxxxxxxxx>
> > cc: Jens Axboe <axboe@xxxxxxxxx>
> > cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
> > cc: linux-s390@xxxxxxxxxxxxxxx
> > cc: netdev@xxxxxxxxxxxxxxx
> > ---
> >   net/smc/af_smc.c    | 29 -----------------------------
> >   net/smc/smc_stats.c |  2 +-
> >   net/smc/smc_stats.h |  1 -
> >   net/smc/smc_tx.c    | 16 ----------------
> >   net/smc/smc_tx.h    |  2 --
> >   5 files changed, 1 insertion(+), 49 deletions(-)
> >
> > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> > index a4cccdfdc00a..d4113c8a7cda 100644
> > --- a/net/smc/af_smc.c
> > +++ b/net/smc/af_smc.c
> > @@ -3125,34 +3125,6 @@ static int smc_ioctl(struct socket *sock, 
> > unsigned int cmd,
> >       return put_user(answ, (int __user *)arg);
> >   }
> >   -static ssize_t smc_sendpage(struct socket *sock, struct page *page,
> > -                int offset, size_t size, int flags)
> > -{
> > -    struct sock *sk = sock->sk;
> > -    struct smc_sock *smc;
> > -    int rc = -EPIPE;
> > -
> > -    smc = smc_sk(sk);
> > -    lock_sock(sk);
> > -    if (sk->sk_state != SMC_ACTIVE) {
> > -        release_sock(sk);
> > -        goto out;
> > -    }
> > -    release_sock(sk);
> > -    if (smc->use_fallback) {
> > -        rc = kernel_sendpage(smc->clcsock, page, offset,
> > -                     size, flags);
> > -    } else {
> > -        lock_sock(sk);
> > -        rc = smc_tx_sendpage(smc, page, offset, size, flags);
> > -        release_sock(sk);
> > -        SMC_STAT_INC(smc, sendpage_cnt);
> > -    }
> > -
> > -out:
> > -    return rc;
> > -}
> > -
> >   /* Map the affected portions of the rmbe into an spd, note the 
> > number of bytes
> >    * to splice in conn->splice_pending, and press 'go'. Delays 
> > consumer cursor
> >    * updates till whenever a respective page has been fully processed.
> > @@ -3224,7 +3196,6 @@ static const struct proto_ops smc_sock_ops = {
> >       .sendmsg    = smc_sendmsg,
> >       .recvmsg    = smc_recvmsg,
> >       .mmap        = sock_no_mmap,
> > -    .sendpage    = smc_sendpage,
> >       .splice_read    = smc_splice_read,
> >   };
> >   diff --git a/net/smc/smc_stats.c b/net/smc/smc_stats.c
> > index e80e34f7ac15..ca14c0f3a07d 100644
> > --- a/net/smc/smc_stats.c
> > +++ b/net/smc/smc_stats.c
> > @@ -227,7 +227,7 @@ static int smc_nl_fill_stats_tech_data(struct 
> > sk_buff *skb,
> >                     SMC_NLA_STATS_PAD))
> >           goto errattr;
> >       if (nla_put_u64_64bit(skb, SMC_NLA_STATS_T_SENDPAGE_CNT,
> > -                  smc_tech->sendpage_cnt,
> > +                  0,
> >                     SMC_NLA_STATS_PAD))
> >           goto errattr;
> >       if (nla_put_u64_64bit(skb, SMC_NLA_STATS_T_CORK_CNT,
> > diff --git a/net/smc/smc_stats.h b/net/smc/smc_stats.h
> > index 84b7ecd8c05c..b60fe1eb37ab 100644
> > --- a/net/smc/smc_stats.h
> > +++ b/net/smc/smc_stats.h
> > @@ -71,7 +71,6 @@ struct smc_stats_tech {
> >       u64            clnt_v2_succ_cnt;
> >       u64            srv_v1_succ_cnt;
> >       u64            srv_v2_succ_cnt;
> > -    u64            sendpage_cnt;
> >       u64            urg_data_cnt;
> >       u64            splice_cnt;
> >       u64            cork_cnt;
> > diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
> > index f4b6a71ac488..d31ce8209fa2 100644
> > --- a/net/smc/smc_tx.c
> > +++ b/net/smc/smc_tx.c
> > @@ -298,22 +298,6 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct 
> > msghdr *msg, size_t len)
> >       return rc;
> >   }
> >   -int smc_tx_sendpage(struct smc_sock *smc, struct page *page, int 
> > offset,
> > -            size_t size, int flags)
> > -{
> > -    struct msghdr msg = {.msg_flags = flags};
> > -    char *kaddr = kmap(page);
> > -    struct kvec iov;
> > -    int rc;
> > -
> > -    iov.iov_base = kaddr + offset;
> > -    iov.iov_len = size;
> > -    iov_iter_kvec(&msg.msg_iter, ITER_SOURCE, &iov, 1, size);
> > -    rc = smc_tx_sendmsg(smc, &msg, size);
> > -    kunmap(page);
> > -    return rc;
> > -}
> > -
> >   /***************************** sndbuf consumer 
> > *******************************/
> >     /* sndbuf consumer: actual data transfer of one target chunk with 
> > ISM write */
> > diff --git a/net/smc/smc_tx.h b/net/smc/smc_tx.h
> > index 34b578498b1f..a59f370b8b43 100644
> > --- a/net/smc/smc_tx.h
> > +++ b/net/smc/smc_tx.h
> > @@ -31,8 +31,6 @@ void smc_tx_pending(struct smc_connection *conn);
> >   void smc_tx_work(struct work_struct *work);
> >   void smc_tx_init(struct smc_sock *smc);
> >   int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t 
> > len);
> > -int smc_tx_sendpage(struct smc_sock *smc, struct page *page, int 
> > offset,
> > -            size_t size, int flags);
> >   int smc_tx_sndbuf_nonempty(struct smc_connection *conn);
> >   void smc_tx_sndbuf_nonfull(struct smc_sock *smc);
> >   void smc_tx_consumer_update(struct smc_connection *conn, bool force);