On Thu, Mar 12, 2020 at 07:37:16PM -0700, Bart Van Assche wrote:
> Move the get_unaligned_be24(), get_unaligned_le24() and
> put_unaligned_le24() definitions from various drivers into
> include/linux/unaligned/generic.h. Add a put_unaligned_be24()
> implementation.
Thank you!
My comments below.
After addressing,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Cc: Keith Busch <kbusch@xxxxxxxxxx>
> Cc: Sagi Grimberg <sagi@xxxxxxxxxxx>
> Cc: Jens Axboe <axboe@xxxxxx>
> Cc: Felipe Balbi <balbi@xxxxxxxxxx>
> Cc: Harvey Harrison <harvey.harrison@xxxxxxxxx>
> Cc: Martin K. Petersen <martin.petersen@xxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: H. Peter Anvin <hpa@xxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
> ---
> drivers/nvme/host/rdma.c | 8 ----
> drivers/nvme/target/rdma.c | 6 ---
> drivers/usb/gadget/function/f_mass_storage.c | 1 +
> drivers/usb/gadget/function/storage_common.h | 5 ---
> include/linux/unaligned/generic.h | 46 ++++++++++++++++++++
> include/target/target_core_backend.h | 6 ---
> 6 files changed, 47 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 3e85c5cacefd..2845118e6e40 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
> @@ -142,14 +142,6 @@ static void nvme_rdma_recv_done(struct ib_cq *cq, struct ib_wc *wc);
> static const struct blk_mq_ops nvme_rdma_mq_ops;
> static const struct blk_mq_ops nvme_rdma_admin_mq_ops;
>
> -/* XXX: really should move to a generic header sooner or later.. */
> -static inline void put_unaligned_le24(u32 val, u8 *p)
> -{
> - *p++ = val;
> - *p++ = val >> 8;
> - *p++ = val >> 16;
> -}
> -
> static inline int nvme_rdma_queue_idx(struct nvme_rdma_queue *queue)
> {
> return queue - queue->ctrl->queues;
> diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c
> index 37d262a65877..8fcede75e02a 100644
> --- a/drivers/nvme/target/rdma.c
> +++ b/drivers/nvme/target/rdma.c
> @@ -143,12 +143,6 @@ static int num_pages(int len)
> return 1 + (((len - 1) & PAGE_MASK) >> PAGE_SHIFT);
> }
>
> -/* XXX: really should move to a generic header sooner or later.. */
> -static inline u32 get_unaligned_le24(const u8 *p)
> -{
> - return (u32)p[0] | (u32)p[1] << 8 | (u32)p[2] << 16;
> -}
> -
> static inline bool nvmet_rdma_need_data_in(struct nvmet_rdma_rsp *rsp)
> {
> return nvme_is_write(rsp->req.cmd) &&
> diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
> index 7c96c4665178..950d2a85f098 100644
> --- a/drivers/usb/gadget/function/f_mass_storage.c
> +++ b/drivers/usb/gadget/function/f_mass_storage.c
> @@ -216,6 +216,7 @@
> #include <linux/freezer.h>
> #include <linux/module.h>
> #include <linux/uaccess.h>
> +#include <asm/unaligned.h>
>
> #include <linux/usb/ch9.h>
> #include <linux/usb/gadget.h>
> diff --git a/drivers/usb/gadget/function/storage_common.h b/drivers/usb/gadget/function/storage_common.h
> index e5e3a2553aaa..bdeb1e233fc9 100644
> --- a/drivers/usb/gadget/function/storage_common.h
> +++ b/drivers/usb/gadget/function/storage_common.h
> @@ -172,11 +172,6 @@ enum data_direction {
> DATA_DIR_NONE
> };
>
> -static inline u32 get_unaligned_be24(u8 *buf)
> -{
> - return 0xffffff & (u32) get_unaligned_be32(buf - 1);
> -}
> -
> static inline struct fsg_lun *fsg_lun_from_dev(struct device *dev)
> {
> return container_of(dev, struct fsg_lun, dev);
> diff --git a/include/linux/unaligned/generic.h b/include/linux/unaligned/generic.h
> index 57d3114656e5..5a0cefda7a13 100644
> --- a/include/linux/unaligned/generic.h
> +++ b/include/linux/unaligned/generic.h
> @@ -2,6 +2,8 @@
> #ifndef _LINUX_UNALIGNED_GENERIC_H
> #define _LINUX_UNALIGNED_GENERIC_H
>
> +#include <linux/types.h>
> +
> /*
> * Cause a link-time error if we try an unaligned access other than
> * 1,2,4 or 8 bytes long
> @@ -66,4 +68,48 @@ extern void __bad_unaligned_access_size(void);
> } \
> (void)0; })
>
> +static inline u32 __get_unaligned_be24(const u8 *p)
> +{
> + return p[0] << 16 | p[1] << 8 | p[2];
> +}
> +
> +static inline u32 get_unaligned_be24(const void *p)
> +{
> + return __get_unaligned_be24(p);
> +}
> +
> +static inline u32 __get_unaligned_le24(const u8 *p)
> +{
> + return p[0] | p[1] << 8 | p[2] << 16;
> +}
> +
> +static inline u32 get_unaligned_le24(const void *p)
> +{
> + return __get_unaligned_le24(p);
> +}
> +
> +static inline void __put_unaligned_be24(u32 val, u8 *p)
const u32 val
> +{
> + *p++ = val >> 16;
> + *p++ = val >> 8;
> + *p++ = val;
> +}
> +
> +static inline void put_unaligned_be24(u32 val, void *p)
Ditto.
> +{
> + __put_unaligned_be24(val, p);
> +}
> +
> +static inline void __put_unaligned_le24(u32 val, u8 *p)
Ditto.
> +{
> + *p++ = val;
> + *p++ = val >> 8;
> + *p++ = val >> 16;
> +}
> +
> +static inline void put_unaligned_le24(u32 val, void *p)
Ditto.
> +{
> + __put_unaligned_le24(val, p);
> +}
> +
> #endif /* _LINUX_UNALIGNED_GENERIC_H */
> diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h
> index 51b6f50eabee..1b752d8ea529 100644
> --- a/include/target/target_core_backend.h
> +++ b/include/target/target_core_backend.h
> @@ -116,10 +116,4 @@ static inline bool target_dev_configured(struct se_device *se_dev)
> return !!(se_dev->dev_flags & DF_CONFIGURED);
> }
>
> -/* Only use get_unaligned_be24() if reading p - 1 is allowed. */
> -static inline uint32_t get_unaligned_be24(const uint8_t *const p)
> -{
> - return get_unaligned_be32(p - 1) & 0xffffffU;
> -}
> -
> #endif /* TARGET_CORE_BACKEND_H */
--
With Best Regards,
Andy Shevchenko
