On Thu, Mar 14, 2019 at 05:09:22PM +0200, Gal Pressman wrote:
> On 14-Mar-19 16:54, Leon Romanovsky wrote:
> > On Thu, Mar 14, 2019 at 01:45:14PM +0200, Gal Pressman wrote:
> >> Add EFA driver generic header file defining driver's device independent
> >> internal data structures and definitions.
> >>
> >> Signed-off-by: Gal Pressman <galpress@xxxxxxxxxx>
> >> Reviewed-by: Steve Wise <swise@xxxxxxxxxxxxxxxxxxxxx>
> >> ---
> >> drivers/infiniband/hw/efa/efa.h | 191 ++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 191 insertions(+)
> >> create mode 100644 drivers/infiniband/hw/efa/efa.h
> >>
> >> diff --git a/drivers/infiniband/hw/efa/efa.h b/drivers/infiniband/hw/efa/efa.h
> >> new file mode 100644
> >> index 000000000000..fac08b0f59df
> >> --- /dev/null
> >> +++ b/drivers/infiniband/hw/efa/efa.h
> >> @@ -0,0 +1,191 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
> >> +/*
> >> + * Copyright 2018-2019 Amazon.com, Inc. or its affiliates. All rights reserved.
> >> + */
> >> +
> >> +#ifndef _EFA_H_
> >> +#define _EFA_H_
> >> +
> >> +#include <linux/bitops.h>
> >> +#include <linux/idr.h>
> >> +#include <linux/interrupt.h>
> >> +#include <linux/pci.h>
> >> +#include <linux/sched.h>
> >> +
> >> +#include <rdma/efa-abi.h>
> >> +#include <rdma/ib_verbs.h>
> >> +
> >> +#include "efa_com_cmd.h"
> >> +
> >> +#define DRV_MODULE_NAME "efa"
> >> +#define DEVICE_NAME "Elastic Fabric Adapter (EFA)"
> >> +
> >> +#define EFA_IRQNAME_SIZE 40
> >> +
> >> +/* 1 for AENQ + ADMIN */
> >> +#define EFA_NUM_MSIX_VEC 1
> >> +#define EFA_MGMNT_MSIX_VEC_IDX 0
> >> +
> >> +#define efa_dbg(_dev, format, ...) \
> >> + dev_dbg(_dev, "(pid %d) %s: " format, current->pid, \
> >> + __func__, ##__VA_ARGS__)
> >> +#define efa_info(_dev, format, ...) \
> >> + dev_info(_dev, "(pid %d) %s: " format, current->pid, \
> >> + __func__, ##__VA_ARGS__)
> >> +#define efa_warn(_dev, format, ...) \
> >> + dev_warn(_dev, "(pid %d) %s: " format, current->pid, \
> >> + __func__, ##__VA_ARGS__)
> >> +#define efa_err(_dev, format, ...) \
> >> + dev_err(_dev, "(pid %d) %s: " format, current->pid, \
> >> + __func__, ##__VA_ARGS__)
> >> +#define efa_err_rl(_dev, format, ...) \
> >> + dev_err_ratelimited(_dev, "(pid %d) %s: " format, current->pid, \
> >> + __func__, ##__VA_ARGS__)
> >
> > Every time when I see such debug prints, it makes me wonder if they
> > actually needed. Anyway "current->pid" will print wrong output for any
> > kernel threads. I know that you are not supporting kverbs, but still
> > don't think that it is right thing to print.
>
> What's the reason pid is wrong for kernel threads?
> I found it quite useful to see the process id while debugging, at least for
> userspace applications. Is there anything other we can use instead of
> current->pid that would work for both?
Kernel threads are running in kernel context, but "current" points
to user context. For example any print from workqueue will display
wrong current-pid.
>
> >
> >> +
> >> +enum {
> >> + EFA_DEVICE_RUNNING_BIT,
> >
> > Doesn't RDMA/core manage the state of device running/not running?
>
> This bit is protecting the driver from handling interrupts before the device
> probe is finished (after request_irq is called). This could happen if the device
> sends a keep-alive AENQ message before probe finish for example.
I arbitrary checked a couple of drivers in drivers/infiniband and they
don't do anything special before/after request_irq. Where can I read
about such need to protect request_irq?
Thanks
Attachment:
signature.asc
Description: PGP signature
