Re: [PATCH] net/mlx4_core: Use min_t instead of if for consistency

2017-05-11 Thread Johannes Thumshirn
On 05/11/2017 10:20 AM, Yuval Shaia wrote:
> + nreq = min_t(nreq, MAX_MSIX);

Ahm...
include/linux/kernel.h +802
#define min_t(type, x, y)   \
__min(type, type,   \
  __UNIQUE_ID(min1_), __UNIQUE_ID(min2_),   \
  x, y)

Did you compile this patch?

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCHv2 3/5] rds: ib: remove redundant ib_dealloc_fmr

2017-03-09 Thread Johannes Thumshirn
On 03/09/2017 10:20 AM, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
> 
> Cc: Joe Jin <joe@oracle.com>
> Cc: Junxiao Bi <junxiao...@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun@oracle.com>
> ---

Reviewed-by: Johannes Thumshirn <jthumsh...@suse.de>


-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

2017-03-09 Thread Johannes Thumshirn
On 03/09/2017 10:26 AM, Reshetova, Elena wrote:
> 
>> On 03/09/2017 08:18 AM, Reshetova, Elena wrote:
>>>> On Mon, Mar 06, 2017 at 04:21:09PM +0200, Elena Reshetova wrote:
>>>>> refcount_t type and corresponding API should be
>>>>> used instead of atomic_t when the variable is used as
>>>>> a reference counter. This allows to avoid accidental
>>>>> refcounter overflows that might lead to use-after-free
>>>>> situations.
>>>>>
>>>>> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
>>>>> Signed-off-by: Hans Liljestrand <ishkam...@gmail.com>
>>>>> Signed-off-by: Kees Cook <keesc...@chromium.org>
>>>>> Signed-off-by: David Windsor <dwind...@gmail.com>
>>>>
>>>> This looks OK to me.
>>>>
>>>> Acked-by: Chris Leech <cle...@redhat.com>
>>>
>>> Thank you for review! Do you have a tree that can take this change?
>>
>> Hi Elena,
>>
>> iscsi like fcoe should go via the SCSI tree.
> 
> Thanks Johannes! Should I resend with "Acked-by" added in order for it to be 
> picked up? 

Yes I think this would be a good way to go.

Byte,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 22/29] drivers, scsi: convert iscsi_task.refcount from atomic_t to refcount_t

2017-03-09 Thread Johannes Thumshirn
On 03/09/2017 08:18 AM, Reshetova, Elena wrote:
>> On Mon, Mar 06, 2017 at 04:21:09PM +0200, Elena Reshetova wrote:
>>> refcount_t type and corresponding API should be
>>> used instead of atomic_t when the variable is used as
>>> a reference counter. This allows to avoid accidental
>>> refcounter overflows that might lead to use-after-free
>>> situations.
>>>
>>> Signed-off-by: Elena Reshetova <elena.reshet...@intel.com>
>>> Signed-off-by: Hans Liljestrand <ishkam...@gmail.com>
>>> Signed-off-by: Kees Cook <keesc...@chromium.org>
>>> Signed-off-by: David Windsor <dwind...@gmail.com>
>>
>> This looks OK to me.
>>
>> Acked-by: Chris Leech <cle...@redhat.com>
> 
> Thank you for review! Do you have a tree that can take this change? 

Hi Elena,

iscsi like fcoe should go via the SCSI tree.

Byte,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 3/5] rds: ib: remove redundant ib_dealloc_fmr

2017-03-09 Thread Johannes Thumshirn
On 03/09/2017 08:26 AM, Zhu Yanjun wrote:
> The function ib_dealloc_fmr will never be called. As such, it should
> be removed.
> 
> Cc: Joe Jin <joe@oracle.com>
> Cc: Junxiao Bi <junxiao...@oracle.com>
> Signed-off-by: Zhu Yanjun <yanjun@oracle.com>
> ---
>  net/rds/ib_fmr.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rds/ib_fmr.c b/net/rds/ib_fmr.c
> index 4fe8f4f..b807df6 100644
> --- a/net/rds/ib_fmr.c
> +++ b/net/rds/ib_fmr.c
> @@ -78,12 +78,10 @@ struct rds_ib_mr *rds_ib_alloc_fmr(struct rds_ib_device 
> *rds_ibdev, int npages)
>   return ibmr;
>  
>  out_no_cigar:
> - if (ibmr) {
> - if (fmr->fmr)
> - ib_dealloc_fmr(fmr->fmr);
> + if (ibmr)
>   kfree(ibmr);

kfree() already does a NULL check (see
http://lxr.free-electrons.com/source/mm/slab.c#L3811) so no need to do
it here.



-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t

2017-03-08 Thread Johannes Thumshirn

On 03/08/2017 02:48 PM, Reshetova, Elena wrote:

On 03/06/2017 03:21 PM, Elena Reshetova wrote:

refcount_t type and corresponding API should be
used instead of atomic_t when the variable is used as
a reference counter. This allows to avoid accidental
refcounter overflows that might lead to use-after-free
situations.


The subject is wrong, should be something like "scsi: libfc convert
fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390.

Other than that
Acked-by: Johannes Thumshirn <j...@kernel.org>


Turns out that it is better that all these patches go through the respective 
maintainer trees, if present.
If I send an updated patch (with subject fixed), could you merge it through 
your tree?


Yes, but this would be the normal scsi tree from Martin and James.

Please include my Ack in the re-sends.

Thanks a lot,
    Johannes


--
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 21/29] drivers, s390: convert fc_fcp_pkt.ref_cnt from atomic_t to refcount_t

2017-03-06 Thread Johannes Thumshirn
On 03/06/2017 03:21 PM, Elena Reshetova wrote:
> refcount_t type and corresponding API should be
> used instead of atomic_t when the variable is used as
> a reference counter. This allows to avoid accidental
> refcounter overflows that might lead to use-after-free
> situations.

The subject is wrong, should be something like "scsi: libfc convert
fc_fcp_pkt.ref_cnt from atomic_t to refcount_t" but not s390.

Other than that
Acked-by: Johannes Thumshirn <j...@kernel.org>

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCHv1] net-next: treewide use is_vlan_dev() helper function.

2017-02-05 Thread Johannes Thumshirn


On 02/04/2017 06:00 PM, Parav Pandit wrote:

This patch makes use of is_vlan_dev() function instead of flag
comparison which is exactly done by is_vlan_dev() helper function.

Signed-off-by: Parav Pandit <pa...@mellanox.com>
Reviewed-by: Daniel Jurgens <dani...@mellanox.com>
---



For drivers/scsi/fcoe/fcoe.c:
Acked-by: Johannes Thumshirn <j...@kernel.org>



Re: [PATCH net] qed: fix old-style function definition

2016-12-16 Thread Johannes Thumshirn
On Fri, Dec 16, 2016 at 09:47:41AM +0100, Arnd Bergmann wrote:
> The newly added file causes a harmless warning, with "make W=1":
> 
> drivers/net/ethernet/qlogic/qed/qed_iscsi.c: In function 'qed_get_iscsi_ops':
> drivers/net/ethernet/qlogic/qed/qed_iscsi.c:1268:29: warning: old-style 
> function definition [-Wold-style-definition]
> 
> This makes it a proper prototype.
> 
> Fixes: fc831825f99e ("qed: Add support for hardware offloaded iSCSI.")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---

Looks good,
Reviewed-by: Johannes Thumshirn <jthumsh...@suse.de>

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [RFC 1/6] qed: Add support for hardware offloaded iSCSI.

2016-10-20 Thread Johannes Thumshirn
Hi Arun,

On Wed, Oct 19, 2016 at 05:14:59PM -0700, Arun Easi wrote:
> Thanks Johannes for the review, please see my response below.
> 

[...]

> > 
> > Why not introduce a small helper like:
> > static inline bool qed_is_iscsi_personality()
> > {
> > return IS_ENABLED(CONFIG_QEDI) && p_hwfn->hw_info.personality ==
> > QED_PCI_ISCSI;
> > }
> 
> I think I can remove the IS_ENABLED() check in places like this
> and have the check contained in header file. qed_iscsi_free()
> already is taken care, if I do the same fore qed_ooo*, I think
> the check would just be "p_hwfn->hw_info.personality ==
> QED_PCI_ISCSI", which would keep it consistent with the other
> areas where similar check is done for other protocols.

Sounds good.

> 

[...]

> > 
> > Is there any chance you could factor out above blocks into own functions so
> > you have
> > 
> > 
> > if (!GET_FIELD(p_ramrod->iscsi.flags,
> >ISCSI_CONN_OFFLOAD_PARAMS_TCP_ON_CHIP_1B)) {
> > qedi_do_stuff_off_chip();
> > else 
> > qedi_do_stuff_on_chip();
> > 
> 
> This function mostly fills data needed for the firmware interface.
> By having all data necessary for the command
> ISCSI_RAMROD_CMD_ID_OFFLOAD_CONN in this function it is easier to
> refer what is being fed to firmware. If you do not have strong
> objections, I would like to keep it this way.

No strong objections, I just don't think these lengthy blocks are readable,
but I don't want to do too much bikeshedding about it.

Thanks,
Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [RFC 2/6] qed: Add iSCSI out of order packet handling.

2016-10-19 Thread Johannes Thumshirn
iscsi_ooo->ooo_isle,
> +p_buffer,
> +QED_OOO_RIGHT_BUF);
> + break;
> + case TCP_EVENT_ADD_ISLE_LEFT:
> + qed_ooo_add_new_buffer(p_hwfn,
> +p_hwfn->p_ooo_info,
> +cid,
> +iscsi_ooo->ooo_isle,
> +p_buffer,
> +QED_OOO_LEFT_BUF);
> + break;
> + case TCP_EVENT_JOIN:
> + qed_ooo_add_new_buffer(p_hwfn,
> +p_hwfn->p_ooo_info,
> +cid,
> +iscsi_ooo->ooo_isle +
> +1,
> +p_buffer,
> +QED_OOO_LEFT_BUF);
> + qed_ooo_join_isles(p_hwfn,
> +p_hwfn->p_ooo_info,
> +cid, iscsi_ooo->ooo_isle);
> + break;
> + case TCP_EVENT_ADD_PEN:
> + num_ooo_add_to_peninsula++;
> + qed_ooo_put_ready_buffer(p_hwfn,
> +  p_hwfn->p_ooo_info,
> +  p_buffer, true);
> + break;
> + }
> + } else {
> + DP_NOTICE(p_hwfn,
> +   "Unexpected event (%d) TX OOO completion\n",
> +   iscsi_ooo->ooo_opcode);
> + }
> + }

Can you factoror the body of that "while(cq_new_idx != cq_old_idx)" loop into
a own function?

>  
> - b_last = list_empty(_rx->active_descq);
> + /* Submit RX buffer here */
> + while ((p_buffer = qed_ooo_get_free_buffer(p_hwfn,
> +p_hwfn->p_ooo_info))) {

This could be an opportunity for a qed_for_each_free_buffer() or maybe even a
qed_ooo_submit_rx_buffers() and qed_ooo_submit_tx_buffers() as this is mostly
duplicate code.

> + rc = qed_ll2_post_rx_buffer(p_hwfn, p_ll2_conn->my_id,
> + p_buffer->rx_buffer_phys_addr,
> + 0, p_buffer, true);
> + if (rc) {
> + qed_ooo_put_free_buffer(p_hwfn, p_hwfn->p_ooo_info,
> + p_buffer);
> + break;
> + }
>   }
> +
> + /* Submit Tx buffers here */
> + while ((p_buffer = qed_ooo_get_ready_buffer(p_hwfn,
> + p_hwfn->p_ooo_info))) {

Ditto.

[...]
> +
> + /* Submit Tx buffers here */
> + while ((p_buffer = qed_ooo_get_ready_buffer(p_hwfn,
> + p_hwfn->p_ooo_info))) {


And here

[...]

> + while ((p_buffer = qed_ooo_get_free_buffer(p_hwfn,
> +p_hwfn->p_ooo_info))) {

[..]

> + while ((p_buffer = qed_ooo_get_free_buffer(p_hwfn,
> +p_hwfn->p_ooo_info))) {

[...]

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [RFC 5/6] qedi: Add support for iSCSI session management.

2016-10-19 Thread Johannes Thumshirn
On Wed, Oct 19, 2016 at 01:01:12AM -0400, manish.rangan...@cavium.com wrote:
> From: Manish Rangankar <manish.rangan...@cavium.com>
> 
> This patch adds support for iscsi_transport LLD Login,
> Logout, NOP-IN/NOP-OUT, Async, Reject PDU processing
> and Firmware async event handling support.
> 
> Signed-off-by: Nilesh Javali <nilesh.jav...@cavium.com>
> Signed-off-by: Adheer Chandravanshi <adheer.chandravan...@qlogic.com>
> Signed-off-by: Chad Dupuis <chad.dup...@cavium.com>
> Signed-off-by: Saurav Kashyap <saurav.kash...@cavium.com>
> Signed-off-by: Arun Easi <arun.e...@cavium.com>
> Signed-off-by: Manish Rangankar <manish.rangan...@cavium.com>
> ---

[...]

> +void qedi_iscsi_unmap_sg_list(struct qedi_cmd *cmd)
> +{
> + struct scsi_cmnd *sc = cmd->scsi_cmd;
> +
> + if (cmd->io_tbl.sge_valid && sc) {
> + scsi_dma_unmap(sc);
> + cmd->io_tbl.sge_valid = 0;
> + }
> +}

Maybe set sge_valid to 0 and then call scsi_dma_unmap(). I don't know if it's
really racy but it looks like it is.

[...]

> +static void qedi_process_text_resp(struct qedi_ctx *qedi,
> +union iscsi_cqe *cqe,
> +struct iscsi_task *task,
> +struct qedi_conn *qedi_conn)
> +{
> + struct iscsi_conn *conn = qedi_conn->cls_conn->dd_data;
> + struct iscsi_session *session = conn->session;
> + struct iscsi_task_context *task_ctx;
> + struct iscsi_text_rsp *resp_hdr_ptr;
> + struct iscsi_text_response_hdr *cqe_text_response;
> + struct qedi_cmd *cmd;
> + int pld_len;
> + u32 *tmp;
> +
> + cmd = (struct qedi_cmd *)task->dd_data;
> + task_ctx = (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
> +   cmd->task_id);

No need to cast here, qedi_get_task_mem() returns void *.

[...]

> + cqe_login_response = >cqe_common.iscsi_hdr.login_response;
> + task_ctx = (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
> +   cmd->task_id);

Same here.

[...]

> + }
> +
> + pbl = (struct scsi_bd *)qedi->bdq_pbl;
> + pbl += (qedi->bdq_prod_idx % qedi->rq_num_entries);
> + pbl->address.hi =
> +   cpu_to_le32((u32)(((u64)(qedi->bdq[idx].buf_dma)) >> 32));
> + pbl->address.lo =
> + cpu_to_le32(((u32)(((u64)(qedi->bdq[idx].buf_dma)) &
> + 0x)));

Is this LISP or C?

> + QEDI_INFO(>dbg_ctx, QEDI_LOG_CONN,
> +   "pbl [0x%p] pbl->address hi [0x%llx] lo [0x%llx] idx [%d]\n",
> +   pbl, pbl->address.hi, pbl->address.lo, idx);
> + pbl->opaque.hi = cpu_to_le32((u32)(((u64)0) >> 32));

Isn't this plain pbl->opaque.hi = 0; ?

> + pbl->opaque.lo = cpu_to_le32(((u32)(((u64)idx) & 0x)));
> +

[...]

> + switch (comp_type) {
> + case ISCSI_CQE_TYPE_SOLICITED:
> + case ISCSI_CQE_TYPE_SOLICITED_WITH_SENSE:
> + fw_task_ctx =
> +   (struct iscsi_task_context *)qedi_get_task_mem(>tasks,
> +   cqe->cqe_solicited.itid);

Again, no cast needed.

[...]

> + writel(*(u32 *), qedi_conn->ep->p_doorbell);
> + /* Make sure fw idx is coherent */
> + wmb();
> + mmiowb();

Isn't either wmb() or mmiowb() enough?

[..]

> +
> + fw_task_ctx =
> +  (struct iscsi_task_context *)qedi_get_task_mem(>tasks, tid);

Cast again.

[...]

> + fw_task_ctx =
> +  (struct iscsi_task_context *)qedi_get_task_mem(>tasks, tid);

^^

[...]

> + fw_task_ctx =
> + (struct iscsi_task_context *)qedi_get_task_mem(>tasks, tid);


[...]

> + fw_task_ctx =
> +   (struct iscsi_task_context *)qedi_get_task_mem(>tasks, tid);
> +

[...]

> +
> + qedi = (struct qedi_ctx *)iscsi_host_priv(shost);

Same goes for iscsi_host_priv();

[...]

> + ret = wait_event_interruptible_timeout(qedi_ep->ofld_wait,
> +((qedi_ep->state ==
> + EP_STATE_OFLDCONN_FAILED) ||
> + (qedi_ep->state ==
> + EP_STATE_OFLDCONN_COMPL)),
> + msecs_to_jiffies(timeout_ms));

Maybe:
#define QEDI_OLDCON_STATE(q) ((q)->state == EP_STATE_OFLDCONN_FAILED || \
(q)->state == EP_STATE_OFLDCONN_CO

Re: [RFC 1/6] qed: Add support for hardware offloaded iSCSI.

2016-10-19 Thread Johannes Thumshirn
ocal_mac_addr_mid))[1] = ucval;
> + ucval = p_conn->local_mac[5];
> + ((u8 *)(_tcp2->local_mac_addr_lo))[0] = ucval;
> + ucval = p_conn->local_mac[4];
> + ((u8 *)(_tcp2->local_mac_addr_lo))[1] = ucval;
> +
> + ucval = p_conn->remote_mac[1];
> + ((u8 *)(_tcp2->remote_mac_addr_hi))[0] = ucval;
> + ucval = p_conn->remote_mac[0];
> + ((u8 *)(_tcp2->remote_mac_addr_hi))[1] = ucval;
> + ucval = p_conn->remote_mac[3];
> + ((u8 *)(_tcp2->remote_mac_addr_mid))[0] = ucval;
> + ucval = p_conn->remote_mac[2];
> + ((u8 *)(_tcp2->remote_mac_addr_mid))[1] = ucval;
> + ucval = p_conn->remote_mac[5];
> + ((u8 *)(_tcp2->remote_mac_addr_lo))[0] = ucval;
> + ucval = p_conn->remote_mac[4];
> + ((u8 *)(_tcp2->remote_mac_addr_lo))[1] = ucval;
> +
> + p_tcp2->vlan_id = cpu_to_le16(p_conn->vlan_id);
> + p_tcp2->flags = p_conn->tcp_flags;
> +
> + p_tcp2->ip_version = p_conn->ip_version;
> + for (i = 0; i < 4; i++) {
> + dval = p_conn->remote_ip[i];
> + p_tcp2->remote_ip[i] = cpu_to_le32(dval);
> + dval = p_conn->local_ip[i];
> + p_tcp2->local_ip[i] = cpu_to_le32(dval);
> + }
> +
> + p_tcp2->flow_label = cpu_to_le32(p_conn->flow_label);
> + p_tcp2->ttl = p_conn->ttl;
> + p_tcp2->tos_or_tc = p_conn->tos_or_tc;
> + p_tcp2->remote_port = cpu_to_le16(p_conn->remote_port);
> + p_tcp2->local_port = cpu_to_le16(p_conn->local_port);
> + p_tcp2->mss = cpu_to_le16(p_conn->mss);
> + p_tcp2->rcv_wnd_scale = p_conn->rcv_wnd_scale;
> + p_tcp2->connect_mode = p_conn->connect_mode;
> + wval = p_conn->syn_ip_payload_length;
> + p_tcp2->syn_ip_payload_length = cpu_to_le16(wval);
> + p_tcp2->syn_phy_addr_lo = DMA_LO_LE(p_conn->syn_phy_addr);
> + p_tcp2->syn_phy_addr_hi = DMA_HI_LE(p_conn->syn_phy_addr);
> + }

Is there any chance you could factor out above blocks into own functions so
you have


if (!GET_FIELD(p_ramrod->iscsi.flags,
   ISCSI_CONN_OFFLOAD_PARAMS_TCP_ON_CHIP_1B)) {
qedi_do_stuff_off_chip();
else 
qedi_do_stuff_on_chip();

> +

[...]

> +static void __iomem *qed_iscsi_get_db_addr(struct qed_hwfn *p_hwfn, u32 cid)
> +{
> + return (u8 __iomem *)p_hwfn->doorbells +
> +  qed_db_addr(cid, DQ_DEMS_LEGACY);
> +}
> +
> +static void __iomem *qed_iscsi_get_primary_bdq_prod(struct qed_hwfn *p_hwfn,
> + u8 bdq_id)
> +{
> + u8 bdq_function_id = ISCSI_BDQ_ID(p_hwfn->port_id);
> +
> + return (u8 __iomem *)p_hwfn->regview + GTT_BAR0_MAP_REG_MSDM_RAM +
> +  MSTORM_SCSI_BDQ_EXT_PROD_OFFSET(bdq_function_id,
> +  bdq_id);
> +}
> +
> +static void __iomem *qed_iscsi_get_secondary_bdq_prod(struct qed_hwfn 
> *p_hwfn,
> +   u8 bdq_id)
> +{
> + u8 bdq_function_id = ISCSI_BDQ_ID(p_hwfn->port_id);
> +
> + return (u8 __iomem *)p_hwfn->regview + GTT_BAR0_MAP_REG_TSDM_RAM +
> +  TSTORM_SCSI_BDQ_EXT_PROD_OFFSET(bdq_function_id,
> +  bdq_id);
> +}

Why are you casting to u8* here, you're returning void*? 

[...]

> +
> + if (tasks) {
> + struct qed_tid_mem *tid_info = kzalloc(sizeof(*tid_info),
> +GFP_KERNEL);
> +
> + if (!tid_info) {
> + DP_NOTICE(cdev,
> +   "Failed to allocate tasks information\n");
> + qed_iscsi_stop(cdev);
> + return -ENOMEM;
> + }
> +
> + rc = qed_cxt_get_tid_mem_info(QED_LEADING_HWFN(cdev),
> +   tid_info);
> + if (rc) {
> + DP_NOTICE(cdev, "Failed to gather task information\n");
> + qed_iscsi_stop(cdev);
> + kfree(tid_info);
> + return rc;
> + }
> +
> + /* Fill task information */
> + tasks->size = tid_info->ti

Re: [RFC 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.

2016-10-19 Thread Johannes Thumshirn
rk_list);
> +
> + set_user_nice(current, -20);
> +
> + while (!kthread_should_stop()) {
> + spin_lock_irqsave(>p_work_lock, flags);
> + while (!list_empty(>work_list)) {
> + list_splice_init(>work_list, _list);
> + spin_unlock_irqrestore(>p_work_lock, flags);
> +
> + list_for_each_entry_safe(work, tmp, _list, list) {
> + list_del_init(>list);
> + qedi_fp_process_cqes(work->qedi, >cqe,
> +  work->que_idx);
> + kfree(work);
> + }
> + spin_lock_irqsave(>p_work_lock, flags);
> + }
> + set_current_state(TASK_INTERRUPTIBLE);
> + spin_unlock_irqrestore(>p_work_lock, flags);
> + schedule();
> + }
> + __set_current_state(TASK_RUNNING);
> +
> + return 0;
> +}

A kthread with prio -20 IRQs turned off looping over a list, what could
possibly go wrong here. I bet you your favorite beverage that this will
cause Soft Lockups when running I/O stress tests BTDT.

[...]

> + if (mode != QEDI_MODE_RECOVERY) {
> + if (iscsi_host_add(qedi->shost, >dev)) {
> + QEDI_ERR(>dbg_ctx,
> +  "Could not add iscsi host\n");
> + rc = -ENOMEM;
> + goto remove_host;
> + }
> +
> + /* Allocate uio buffers */
> + rc = qedi_alloc_uio_rings(qedi);
> + if (rc) {
> + QEDI_ERR(>dbg_ctx,
> +  "UIO alloc ring failed err=%d\n", rc);
> + goto remove_host;
> + }
> +
> + rc = qedi_init_uio(qedi);
> + if (rc) {
> + QEDI_ERR(>dbg_ctx,
> +  "UIO init failed, err=%d\n", rc);
> + goto free_uio;
> + }
> +
> + /* host the array on iscsi_conn */
> + rc = qedi_setup_cid_que(qedi);
> + if (rc) {
> + QEDI_ERR(>dbg_ctx,
> +  "Could not setup cid que\n");
> + goto free_uio;
> + }
> +
> + rc = qedi_cm_alloc_mem(qedi);
> + if (rc) {
> + QEDI_ERR(>dbg_ctx,
> +  "Could not alloc cm memory\n");
> + goto free_cid_que;
> + }
> +
> + rc = qedi_alloc_itt(qedi);
> + if (rc) {
> + QEDI_ERR(>dbg_ctx,
> +  "Could not alloc itt memory\n");
> + goto free_cid_que;
> + }
> +
> + sprintf(host_buf, "host_%d", qedi->shost->host_no);
> + qedi->tmf_thread = create_singlethread_workqueue(host_buf);
> + if (!qedi->tmf_thread) {
> + QEDI_ERR(>dbg_ctx,
> +  "Unable to start tmf thread!\n");
> + rc = -ENODEV;
> + goto free_cid_que;
> + }
> +
> + sprintf(host_buf, "qedi_ofld%d", qedi->shost->host_no);
> + qedi->offload_thread = create_workqueue(host_buf);
> + if (!qedi->offload_thread) {
> + QEDI_ERR(>dbg_ctx,
> +  "Unable to start offload thread!\n");
> + rc = -ENODEV;
> + goto free_cid_que;
> + }
> +
> + /* F/w needs 1st task context memory entry for performance */
> + set_bit(QEDI_RESERVE_TASK_ID, qedi->task_idx_map);
> + atomic_set(>num_offloads, 0);
> + }
> +
> + QEDI_INFO(>dbg_ctx, QEDI_LOG_INFO,
> +   "QLogic FastLinQ iSCSI Module qedi %s, FW %d.%d.%d.%d\n",
> +   QEDI_MODULE_VERSION, FW_MAJOR_VERSION, FW_MINOR_VERSION,
> +FW_REVISION_VERSION, FW_ENGINEERING_VERSION);
> + return 0;

Please put the QEDI_INFO() above the if and invert the condition.


Thanks,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes

2016-09-30 Thread Johannes Thumshirn
On Fri, Sep 30, 2016 at 03:56:45PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 9/30/2016 3:11 PM, Johannes Thumshirn wrote:
> 
> > The call to krealloc() in wsm_buf_reserve() directly assigns the newly
> > returned memory to buf->begin. This is all fine except when krealloc()
> > failes we loose the ability to free the old memory pointed to by
> 
>Fails.
> 
> > buf->begin. If we just create a temporary variable to assign memory to
> > and assign the memory to it we can mitigate the memory leak.
> > 
> > Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
> > ---
> >  drivers/net/wireless/st/cw1200/wsm.c | 16 +---
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/st/cw1200/wsm.c 
> > b/drivers/net/wireless/st/cw1200/wsm.c
> > index 680d60e..12fad99 100644
> > --- a/drivers/net/wireless/st/cw1200/wsm.c
> > +++ b/drivers/net/wireless/st/cw1200/wsm.c
> > @@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, 
> > size_t extra_size)
> >  {
> > size_t pos = buf->data - buf->begin;
> > size_t size = pos + extra_size;
> > +   u8 *tmp;
> > 
> > size = round_up(size, FWLOAD_BLOCK_SIZE);
> > 
> > -   buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > -   if (buf->begin) {
> > -   buf->data = >begin[pos];
> > -   buf->end = >begin[size];
> > -       return 0;
> > -   } else {
> > -   buf->end = buf->data = buf->begin;
> > +   tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > +   if (tmp) {
> 
>!tmp, you mean?

Yes, I've already sent out a v2.

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


[PATCH v2] cw1200: Don't leak memory if krealloc failes

2016-09-30 Thread Johannes Thumshirn
The call to krealloc() in wsm_buf_reserve() directly assigns the newly
returned memory to buf->begin. This is all fine except when krealloc()
failes we loose the ability to free the old memory pointed to by
buf->begin. If we just create a temporary variable to assign memory to
and assign the memory to it we can mitigate the memory leak.

Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
Cc: Johannes Berg <johan...@sipsolutions.net>
---
 drivers/net/wireless/st/cw1200/wsm.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

Changes from v1:
* Do the correct check for a failed re-allocation (Johannes Berg)

diff --git a/drivers/net/wireless/st/cw1200/wsm.c 
b/drivers/net/wireless/st/cw1200/wsm.c
index 680d60e..ffb245e 100644
--- a/drivers/net/wireless/st/cw1200/wsm.c
+++ b/drivers/net/wireless/st/cw1200/wsm.c
@@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t 
extra_size)
 {
size_t pos = buf->data - buf->begin;
size_t size = pos + extra_size;
+   u8 *tmp;
 
size = round_up(size, FWLOAD_BLOCK_SIZE);
 
-   buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
-   if (buf->begin) {
-   buf->data = >begin[pos];
-   buf->end = >begin[size];
-   return 0;
-   } else {
-   buf->end = buf->data = buf->begin;
+   tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
+   if (!tmp) {
+   wsm_buf_deinit(buf);
return -ENOMEM;
}
+
+   buf->begin = tmp;
+   buf->data = >begin[pos];
+   buf->end = >begin[size];
+   return 0;
 }
-- 
1.8.5.6



Re: [PATCH 1/3] cw1200: Don't leak memory if krealloc failes

2016-09-30 Thread Johannes Thumshirn
On Fri, Sep 30, 2016 at 02:29:39PM +0200, Johannes Berg wrote:
> 
> > +   tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
> > +   if (tmp) {
> > 
> I think that check is inverted?
> 
> johannes

Fu.. you're right, of cause it's !tmp.

I'll resend.

Thanks,
    Johannes

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


[PATCH 1/3] cw1200: Don't leak memory if krealloc failes

2016-09-30 Thread Johannes Thumshirn
The call to krealloc() in wsm_buf_reserve() directly assigns the newly
returned memory to buf->begin. This is all fine except when krealloc()
failes we loose the ability to free the old memory pointed to by
buf->begin. If we just create a temporary variable to assign memory to
and assign the memory to it we can mitigate the memory leak.

Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
---
 drivers/net/wireless/st/cw1200/wsm.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/wsm.c 
b/drivers/net/wireless/st/cw1200/wsm.c
index 680d60e..12fad99 100644
--- a/drivers/net/wireless/st/cw1200/wsm.c
+++ b/drivers/net/wireless/st/cw1200/wsm.c
@@ -1807,16 +1807,18 @@ static int wsm_buf_reserve(struct wsm_buf *buf, size_t 
extra_size)
 {
size_t pos = buf->data - buf->begin;
size_t size = pos + extra_size;
+   u8 *tmp;
 
size = round_up(size, FWLOAD_BLOCK_SIZE);
 
-   buf->begin = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
-   if (buf->begin) {
-   buf->data = >begin[pos];
-   buf->end = >begin[size];
-   return 0;
-   } else {
-   buf->end = buf->data = buf->begin;
+   tmp = krealloc(buf->begin, size, GFP_KERNEL | GFP_DMA);
+   if (tmp) {
+   wsm_buf_deinit(buf);
return -ENOMEM;
}
+
+   buf->begin = tmp;
+   buf->data = >begin[pos];
+   buf->end = >begin[size];
+   return 0;
 }
-- 
1.8.5.6



Re: [PATCH v3 0/6] Introduce pci_(request|release)_(mem|io)_regions

2016-06-08 Thread Johannes Thumshirn
On Tue, Jun 07, 2016 at 09:44:00AM +0200, Johannes Thumshirn wrote:
> The first patch in this series introduces the following 4 helper functions to
> the PCI core:
> 
> * pci_request_mem_regions()
> * pci_request_io_regions()
> * pci_release_mem_regions()
> * pci_release_io_regions()
> 
> which encapsulate the request and release of a PCI device's memory or I/O
> bars.
> 
> The subsequent patches convert the drivers, which use the
> pci_request_selected_regions(pdev, 
>   pci_select_bars(pdev, IORESOURCE_MEM), name); 
> and similar pattern to use the new interface.
> 
> This was suggested by Christoph Hellwig in
> http://lists.infradead.org/pipermail/linux-nvme/2016-May/004570.html and
> tested on kernel v4.6 with NVMe.
> 

Btw, as I've seen already Jeff applying the Intel Ethernet patch to his
tree, I think this should go via the PCI tree as the build dependency is
the PCI patch.

Thanks,
Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


[PATCH v3 0/6] Introduce pci_(request|release)_(mem|io)_regions

2016-06-07 Thread Johannes Thumshirn
The first patch in this series introduces the following 4 helper functions to
the PCI core:

* pci_request_mem_regions()
* pci_request_io_regions()
* pci_release_mem_regions()
* pci_release_io_regions()

which encapsulate the request and release of a PCI device's memory or I/O
bars.

The subsequent patches convert the drivers, which use the
pci_request_selected_regions(pdev, 
pci_select_bars(pdev, IORESOURCE_MEM), name); 
and similar pattern to use the new interface.

This was suggested by Christoph Hellwig in
http://lists.infradead.org/pipermail/linux-nvme/2016-May/004570.html and
tested on kernel v4.6 with NVMe.

The conversion of the drivers has been performed by the following coccinelle
spatch:
// IORESOURCE_MEM
@@
expression err, pdev, name;
@@

- err = pci_request_selected_regions(pdev, pci_select_bars(pdev, 
IORESOURCE_MEM), name);
+ err = pci_request_mem_regions(pdev, name);

@@
expression pdev;
@@
- pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_MEM));
+ pci_release_mem_regions(pdev);

@@
expression err, pdev, name;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_MEM);
...
- err = pci_request_selected_regions(pdev, bars, name);
+ err = pci_request_mem_regions(pdev, name);

@@
expression pdev;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_MEM);
...
- pci_release_selected_regions(pdev, bars);
+ pci_release_mem_regions(pdev);

// IORESOURCE_IO
@@
expression err, pdev, name;
@@

- err = pci_request_selected_regions(pdev, pci_select_bars(pdev,
IORESOURCE_IO), name);
+ err = pci_request_io_regions(pdev, name);

@@
expression pdev;
@@
- pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_IO));
+ pci_release_io_regions(pdev);

@@
expression err, pdev, name;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_IO);
...
- err = pci_request_selected_regions(pdev, bars, name);
+ err = pci_request_io_regions(pdev, name);

@@
expression pdev;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_IO);
...
- pci_release_selected_regions(pdev, bars);
+ pci_release_io_regions(pdev);

Changes since v2:
* Fixed compilation error on platforms with CONFIG_PCI=n
* Added Jeff's Acked-by on the Intel ethernet patch
* Added Dick's Acked-by on the lpfc patch

Changes since v1:
* Fixed indendatoin in pci.h patch to not cross the 80 chars boundary.
* Split Ethernet patches into two, one for Atheros and one for Intel drivers.
* Correctly named lpfc patch.
* Converted init-path of lpfc driver as well.
* Added Reviewed-by tags were appropriate.

Johannes Thumshirn (6):
  PCI: Add helpers to request/release memory and I/O regions
  NVMe: Use pci_(request|release)_mem_regions
  lpfc: Use pci_(request|release)_mem_regions
  GenWQE: Use pci_(request|release)_mem_regions
  ethernet/intel: Use pci_(request|release)_mem_regions
  alx: Use pci_(request|release)_mem_regions

 drivers/misc/genwqe/card_base.c   | 13 +
 drivers/net/ethernet/atheros/alx/main.c   | 12 +---
 drivers/net/ethernet/intel/e1000e/netdev.c|  6 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 +++
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 +++--
 drivers/net/ethernet/intel/igb/igb_main.c | 10 +++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 +++--
 drivers/nvme/host/pci.c   | 10 +++---
 drivers/scsi/lpfc/lpfc_init.c | 15 --
 include/linux/pci.h   | 28 +++
 10 files changed, 59 insertions(+), 64 deletions(-)

Cc: Christoph Hellwig <h...@infradead.org>
Cc: Keith Busch <keith.bu...@intel.com>
Cc: Jens Axboe <ax...@fb.com>
Cc: linux-n...@lists.infradead.org
Cc: James Smart <james.sm...@avagotech.com>
Cc: Dick Kennedy <dick.kenn...@avagotech.com>
Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
Cc: linux-s...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Frank Haverkamp <ha...@linux.vnet.ibm.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Jay Cliburn <jclib...@gmail.com>
Cc: Chris Snook <chris.sn...@gmail.com>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
-- 
1.8.5.6



[PATCH v3 6/6] alx: Use pci_(request|release)_mem_regions

2016-06-07 Thread Johannes Thumshirn
Now that we do have pci_request_mem_regions() and pci_release_mem_regions() at
hand, use it in the ethernet drivers.

Suggested-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
Cc: Jay Cliburn <jclib...@gmail.com>
Cc: Chris Snook <chris.sn...@gmail.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
---
 drivers/net/ethernet/atheros/alx/main.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/main.c 
b/drivers/net/ethernet/atheros/alx/main.c
index 55b118e..d2363de 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1238,7 +1238,7 @@ static int alx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
struct alx_priv *alx;
struct alx_hw *hw;
bool phy_configured;
-   int bars, err;
+   int err;
 
err = pci_enable_device_mem(pdev);
if (err)
@@ -1258,11 +1258,10 @@ static int alx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
}
}
 
-   bars = pci_select_bars(pdev, IORESOURCE_MEM);
-   err = pci_request_selected_regions(pdev, bars, alx_drv_name);
+   err = pci_request_mem_regions(pdev, alx_drv_name);
if (err) {
dev_err(>dev,
-   "pci_request_selected_regions failed(bars:%d)\n", bars);
+   "pci_request_mem_regions failed\n");
goto out_pci_disable;
}
 
@@ -1388,7 +1387,7 @@ out_unmap:
 out_free_netdev:
free_netdev(netdev);
 out_pci_release:
-   pci_release_selected_regions(pdev, bars);
+   pci_release_mem_regions(pdev);
 out_pci_disable:
pci_disable_device(pdev);
return err;
@@ -1407,8 +1406,7 @@ static void alx_remove(struct pci_dev *pdev)
 
unregister_netdev(alx->dev);
iounmap(hw->hw_addr);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
-- 
1.8.5.6



[PATCH v3 5/6] ethernet/intel: Use pci_(request|release)_mem_regions

2016-06-07 Thread Johannes Thumshirn
Now that we do have pci_request_mem_regions() and pci_release_mem_regions() at
hand, use it in the Intel ethernet drivers.

Suggested-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
Acked-by: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
---
 drivers/net/ethernet/intel/e1000e/netdev.c|  6 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 +++
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 +++--
 drivers/net/ethernet/intel/igb/igb_main.c | 10 +++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 +++--
 5 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9b4ec13..ecd8d15 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7284,8 +7284,7 @@ err_flashmap:
 err_ioremap:
free_netdev(netdev);
 err_alloc_etherdev:
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -7352,8 +7351,7 @@ static void e1000_remove(struct pci_dev *pdev)
if ((adapter->hw.flash_address) &&
(adapter->hw.mac.type < e1000_pch_spt))
iounmap(adapter->hw.flash_address);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
free_netdev(netdev);
 
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c 
b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 4eb7a6f..ad28e87 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1940,10 +1940,7 @@ static int fm10k_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
goto err_dma;
}
 
-   err = pci_request_selected_regions(pdev,
-  pci_select_bars(pdev,
-  IORESOURCE_MEM),
-  fm10k_driver_name);
+   err = pci_request_mem_regions(pdev, fm10k_driver_name);
if (err) {
dev_err(>dev,
"pci_request_selected_regions failed: %d\n", err);
@@ -2034,8 +2031,7 @@ err_sw_init:
 err_ioremap:
free_netdev(netdev);
 err_alloc_netdev:
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -2086,8 +2082,7 @@ static void fm10k_remove(struct pci_dev *pdev)
 
free_netdev(netdev);
 
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3449129..37592b1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -10779,8 +10779,7 @@ static int i40e_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
}
 
/* set up pci connections */
-   err = pci_request_selected_regions(pdev, pci_select_bars(pdev,
-  IORESOURCE_MEM), i40e_driver_name);
+   err = pci_request_mem_regions(pdev, i40e_driver_name);
if (err) {
dev_info(>dev,
 "pci_request_selected_regions failed %d\n", err);
@@ -11277,8 +11276,7 @@ err_ioremap:
kfree(pf);
 err_pf_alloc:
pci_disable_pcie_error_reporting(pdev);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -11387,8 +11385,7 @@ static void i40e_remove(struct pci_dev *pdev)
 
iounmap(hw->hw_addr);
kfree(pf);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index 55a1405c..466087f 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2288,9 +2288,7

[PATCH v2 5/6] ethernet/intel: Use pci_(request|release)_mem_regions

2016-06-02 Thread Johannes Thumshirn
Now that we do have pci_request_mem_regions() and pci_release_mem_regions() at
hand, use it in the Intel ethernet drivers.

Suggested-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
---
 drivers/net/ethernet/intel/e1000e/netdev.c|  6 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 +++
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 +++--
 drivers/net/ethernet/intel/igb/igb_main.c | 10 +++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 +++--
 5 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9b4ec13..ecd8d15 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7284,8 +7284,7 @@ err_flashmap:
 err_ioremap:
free_netdev(netdev);
 err_alloc_etherdev:
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -7352,8 +7351,7 @@ static void e1000_remove(struct pci_dev *pdev)
if ((adapter->hw.flash_address) &&
(adapter->hw.mac.type < e1000_pch_spt))
iounmap(adapter->hw.flash_address);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
free_netdev(netdev);
 
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c 
b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 4eb7a6f..ad28e87 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1940,10 +1940,7 @@ static int fm10k_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
goto err_dma;
}
 
-   err = pci_request_selected_regions(pdev,
-  pci_select_bars(pdev,
-  IORESOURCE_MEM),
-  fm10k_driver_name);
+   err = pci_request_mem_regions(pdev, fm10k_driver_name);
if (err) {
dev_err(>dev,
"pci_request_selected_regions failed: %d\n", err);
@@ -2034,8 +2031,7 @@ err_sw_init:
 err_ioremap:
free_netdev(netdev);
 err_alloc_netdev:
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -2086,8 +2082,7 @@ static void fm10k_remove(struct pci_dev *pdev)
 
free_netdev(netdev);
 
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
 
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c 
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3449129..37592b1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -10779,8 +10779,7 @@ static int i40e_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
}
 
/* set up pci connections */
-   err = pci_request_selected_regions(pdev, pci_select_bars(pdev,
-  IORESOURCE_MEM), i40e_driver_name);
+   err = pci_request_mem_regions(pdev, i40e_driver_name);
if (err) {
dev_info(>dev,
 "pci_request_selected_regions failed %d\n", err);
@@ -11277,8 +11276,7 @@ err_ioremap:
kfree(pf);
 err_pf_alloc:
pci_disable_pcie_error_reporting(pdev);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -11387,8 +11385,7 @@ static void i40e_remove(struct pci_dev *pdev)
 
iounmap(hw->hw_addr);
kfree(pf);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c 
b/drivers/net/ethernet/intel/igb/igb_main.c
index 55a1405c..466087f 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2288,9 +2288,7 @@ static int igb_probe(struct pci_dev *pdev, co

[PATCH v2 6/6] alx: Use pci_(request|release)_mem_regions

2016-06-02 Thread Johannes Thumshirn
Now that we do have pci_request_mem_regions() and pci_release_mem_regions() at
hand, use it in the ethernet drivers.

Suggested-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
Cc: Jay Cliburn <jclib...@gmail.com>
Cc: Chris Snook <chris.sn...@gmail.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
---
 drivers/net/ethernet/atheros/alx/main.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/main.c 
b/drivers/net/ethernet/atheros/alx/main.c
index 55b118e..d2363de 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1238,7 +1238,7 @@ static int alx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
struct alx_priv *alx;
struct alx_hw *hw;
bool phy_configured;
-   int bars, err;
+   int err;
 
err = pci_enable_device_mem(pdev);
if (err)
@@ -1258,11 +1258,10 @@ static int alx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
}
}
 
-   bars = pci_select_bars(pdev, IORESOURCE_MEM);
-   err = pci_request_selected_regions(pdev, bars, alx_drv_name);
+   err = pci_request_mem_regions(pdev, alx_drv_name);
if (err) {
dev_err(>dev,
-   "pci_request_selected_regions failed(bars:%d)\n", bars);
+   "pci_request_mem_regions failed\n");
goto out_pci_disable;
}
 
@@ -1388,7 +1387,7 @@ out_unmap:
 out_free_netdev:
free_netdev(netdev);
 out_pci_release:
-   pci_release_selected_regions(pdev, bars);
+   pci_release_mem_regions(pdev);
 out_pci_disable:
pci_disable_device(pdev);
return err;
@@ -1407,8 +1406,7 @@ static void alx_remove(struct pci_dev *pdev)
 
unregister_netdev(alx->dev);
iounmap(hw->hw_addr);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
-- 
1.8.5.6



[PATCH v2 0/6] Introduce pci_(request|release)_(mem|io)_regions

2016-06-02 Thread Johannes Thumshirn
The first patch in this series introduces the following 4 helper functions to
the PCI core:

* pci_request_mem_regions()
* pci_request_io_regions()
* pci_release_mem_regions()
* pci_release_io_regions()

which encapsulate the request and release of a PCI device's memory or I/O
bars.

The subsequent patches convert the drivers, which use the
pci_request_selected_regions(pdev, 
pci_select_bars(pdev, IORESOURCE_MEM), name); 
and similar pattern to use the new interface.

This was suggested by Christoph Hellwig in
http://lists.infradead.org/pipermail/linux-nvme/2016-May/004570.html and
tested on kernel v4.6 with NVMe.

The conversion of the drivers has been performed by the following coccinelle
spatch:
// IORESOURCE_MEM
@@
expression err, pdev, name;
@@

- err = pci_request_selected_regions(pdev, pci_select_bars(pdev, 
IORESOURCE_MEM), name);
+ err = pci_request_mem_regions(pdev, name);

@@
expression pdev;
@@
- pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_MEM));
+ pci_release_mem_regions(pdev);

@@
expression err, pdev, name;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_MEM);
...
- err = pci_request_selected_regions(pdev, bars, name);
+ err = pci_request_mem_regions(pdev, name);

@@
expression pdev;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_MEM);
...
- pci_release_selected_regions(pdev, bars);
+ pci_release_mem_regions(pdev);

// IORESOURCE_IO
@@
expression err, pdev, name;
@@

- err = pci_request_selected_regions(pdev, pci_select_bars(pdev,
IORESOURCE_IO), name);
+ err = pci_request_io_regions(pdev, name);

@@
expression pdev;
@@
- pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_IO));
+ pci_release_io_regions(pdev);

@@
expression err, pdev, name;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_IO);
...
- err = pci_request_selected_regions(pdev, bars, name);
+ err = pci_request_io_regions(pdev, name);

@@
expression pdev;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_IO);
...
- pci_release_selected_regions(pdev, bars);
+ pci_release_io_regions(pdev);

Changes since v1:
* Fixed indendatoin in pci.h patch to not cross the 80 chars boundary.
* Split Ethernet patches into two, one for Atheros and one for Intel drivers.
* Correctly named lpfc patch.
* Converted init-path of lpfc driver as well.
* Added Reviewed-by tags were appropriate.

Johannes Thumshirn (6):
  PCI: Add helpers to request/release memory and I/O regions
  NVMe: Use pci_(request|release)_mem_regions
  lpfc: Use pci_(request|release)_mem_regions
  GenWQE: Use pci_(request|release)_mem_regions
  ethernet/intel: Use pci_(request|release)_mem_regions
  alx: Use pci_(request|release)_mem_regions

 drivers/misc/genwqe/card_base.c   | 13 +
 drivers/net/ethernet/atheros/alx/main.c   | 12 +---
 drivers/net/ethernet/intel/e1000e/netdev.c|  6 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 +++
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 +++--
 drivers/net/ethernet/intel/igb/igb_main.c | 10 +++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 +++--
 drivers/nvme/host/pci.c   | 10 +++---
 drivers/scsi/lpfc/lpfc_init.c | 15 --
 include/linux/pci.h   | 28 +++
 10 files changed, 59 insertions(+), 64 deletions(-)

Cc: Christoph Hellwig <h...@infradead.org>
Cc: Keith Busch <keith.bu...@intel.com>
Cc: Jens Axboe <ax...@fb.com>
Cc: linux-n...@lists.infradead.org
Cc: James Smart <james.sm...@avagotech.com>
Cc: Dick Kennedy <dick.kenn...@avagotech.com>
Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
Cc: linux-s...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Frank Haverkamp <ha...@linux.vnet.ibm.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Jay Cliburn <jclib...@gmail.com>
Cc: Chris Snook <chris.sn...@gmail.com>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
-- 
1.8.5.6



Re: [PATCH 5/5] ethernet: Use pci_(request|release)_mem_regions

2016-06-01 Thread Johannes Thumshirn
On Wed, Jun 01, 2016 at 07:56:45AM -0700, Jeff Kirsher wrote:
> On Wed, 2016-06-01 at 10:51 +0200, Johannes Thumshirn wrote:
> > On Wed, Jun 01, 2016 at 12:59:56AM -0700, Christoph Hellwig wrote:
> > > On Tue, May 31, 2016 at 02:05:13PM +0200, Johannes Thumshirn wrote:
> > > > Now that we do have pci_request_mem_regions() and
> > pci_release_mem_regions() at
> > > > hand, use it in the ethernet drivers.
> > > 
> > > This should probably be one patch per driver.
> > 
> > I though if I do one patch per subsystem it'll be less a hassle for the
> > individual maintainers, but if the netdev people want it as split up,
> > I'll be
> > doing it of cause.
> 
> Since almost all the changes are to Intel wired LAN drivers, if you just
> split off the atheros change into a separate patch, I would be happy.  Then
> you could keep just one patch to change all the Intel drivers.

That should be doable. I'll send a v2 with all requested changes tomorrow.

-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


Re: [PATCH 5/5] ethernet: Use pci_(request|release)_mem_regions

2016-06-01 Thread Johannes Thumshirn
On Wed, Jun 01, 2016 at 12:59:56AM -0700, Christoph Hellwig wrote:
> On Tue, May 31, 2016 at 02:05:13PM +0200, Johannes Thumshirn wrote:
> > Now that we do have pci_request_mem_regions() and pci_release_mem_regions() 
> > at
> > hand, use it in the ethernet drivers.
> 
> This should probably be one patch per driver.

I though if I do one patch per subsystem it'll be less a hassle for the
individual maintainers, but if the netdev people want it as split up, I'll be
doing it of cause.

    Johannes
-- 
Johannes Thumshirn  Storage
jthumsh...@suse.de+49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850


[PATCH 5/5] ethernet: Use pci_(request|release)_mem_regions

2016-05-31 Thread Johannes Thumshirn
Now that we do have pci_request_mem_regions() and pci_release_mem_regions() at
hand, use it in the ethernet drivers.

Suggested-by: Christoph Hellwig <h...@infradead.org>
Signed-off-by: Johannes Thumshirn <jthumsh...@suse.de>
Cc: Jay Cliburn <jclib...@gmail.com>
Cc: Chris Snook <chris.sn...@gmail.com>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
---
 drivers/net/ethernet/atheros/alx/main.c   | 12 +---
 drivers/net/ethernet/intel/e1000e/netdev.c|  6 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 +++
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 +++--
 drivers/net/ethernet/intel/igb/igb_main.c | 10 +++---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 +++--
 6 files changed, 19 insertions(+), 38 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/main.c 
b/drivers/net/ethernet/atheros/alx/main.c
index 55b118e..d2363de 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1238,7 +1238,7 @@ static int alx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
struct alx_priv *alx;
struct alx_hw *hw;
bool phy_configured;
-   int bars, err;
+   int err;
 
err = pci_enable_device_mem(pdev);
if (err)
@@ -1258,11 +1258,10 @@ static int alx_probe(struct pci_dev *pdev, const struct 
pci_device_id *ent)
}
}
 
-   bars = pci_select_bars(pdev, IORESOURCE_MEM);
-   err = pci_request_selected_regions(pdev, bars, alx_drv_name);
+   err = pci_request_mem_regions(pdev, alx_drv_name);
if (err) {
dev_err(>dev,
-   "pci_request_selected_regions failed(bars:%d)\n", bars);
+   "pci_request_mem_regions failed\n");
goto out_pci_disable;
}
 
@@ -1388,7 +1387,7 @@ out_unmap:
 out_free_netdev:
free_netdev(netdev);
 out_pci_release:
-   pci_release_selected_regions(pdev, bars);
+   pci_release_mem_regions(pdev);
 out_pci_disable:
pci_disable_device(pdev);
return err;
@@ -1407,8 +1406,7 @@ static void alx_remove(struct pci_dev *pdev)
 
unregister_netdev(alx->dev);
iounmap(hw->hw_addr);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
pci_disable_pcie_error_reporting(pdev);
pci_disable_device(pdev);
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c 
b/drivers/net/ethernet/intel/e1000e/netdev.c
index 9b4ec13..ecd8d15 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7284,8 +7284,7 @@ err_flashmap:
 err_ioremap:
free_netdev(netdev);
 err_alloc_etherdev:
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -7352,8 +7351,7 @@ static void e1000_remove(struct pci_dev *pdev)
if ((adapter->hw.flash_address) &&
(adapter->hw.mac.type < e1000_pch_spt))
iounmap(adapter->hw.flash_address);
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 
free_netdev(netdev);
 
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c 
b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 4eb7a6f..ad28e87 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -1940,10 +1940,7 @@ static int fm10k_probe(struct pci_dev *pdev, const 
struct pci_device_id *ent)
goto err_dma;
}
 
-   err = pci_request_selected_regions(pdev,
-  pci_select_bars(pdev,
-  IORESOURCE_MEM),
-  fm10k_driver_name);
+   err = pci_request_mem_regions(pdev, fm10k_driver_name);
if (err) {
dev_err(>dev,
"pci_request_selected_regions failed: %d\n", err);
@@ -2034,8 +2031,7 @@ err_sw_init:
 err_ioremap:
free_netdev(netdev);
 err_alloc_netdev:
-   pci_release_selected_regions(pdev,
-pci_select_bars(pdev, IORESOURCE_MEM));
+   pci_release_mem_regions(pdev);
 err_pci_reg:
 err_dma:
pci_disable_device(pdev);
@@ -2086,8 +2082,7 @@ static void fm10k_remove(struct pci_dev *pdev)
 
free_netdev(netdev);
 
-   pci_release_selected_regions(pdev,
-  

[PATCH 0/5] Introduce pci_(request|release)_(mem|io)_regions

2016-05-31 Thread Johannes Thumshirn
The first patch in this series introduces the following 4 helper functions to
the PCI core:

* pci_request_mem_regions()
* pci_request_io_regions()
* pci_release_mem_regions()
* pci_release_io_regions()

which encapsulate the request and release of a PCI device's memory or I/O
bars.

The subsequent patches convert the drivers, which use the
pci_request_selected_regions(pdev, 
pci_select_bars(pdev, IORESOURCE_MEM), name); 
and similar pattern to use the new interface.

This was suggested by Christoph Hellwig in
http://lists.infradead.org/pipermail/linux-nvme/2016-May/004570.html and
tested on kernel v4.6 with NVMe.

The conversion of the drivers has been performed by the following coccinelle
spatch:
// IORESOURCE_MEM
@@
expression err, pdev, name;
@@

- err = pci_request_selected_regions(pdev, pci_select_bars(pdev,
IORESOURCE_MEM), name);
+ err = pci_request_mem_regions(pdev, name);

@@
expression pdev;
@@
- pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_MEM));
+ pci_release_mem_regions(pdev);

@@
expression err, pdev, name;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_MEM);
...
- err = pci_request_selected_regions(pdev, bars, name);
+ err = pci_request_mem_regions(pdev, name);

@@
expression pdev;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_MEM);
...
- pci_release_selected_regions(pdev, bars);
+ pci_release_mem_regions(pdev);

// IORESOURCE_IO
@@
expression err, pdev, name;
@@

- err = pci_request_selected_regions(pdev, pci_select_bars(pdev,
IORESOURCE_IO), name);
+ err = pci_request_io_regions(pdev, name);

@@
expression pdev;
@@
- pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_IO));
+ pci_release_io_regions(pdev);

@@
expression err, pdev, name;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_IO);
...
- err = pci_request_selected_regions(pdev, bars, name);
+ err = pci_request_io_regions(pdev, name);

@@
expression pdev;
identifier bars;
@@
- bars = pci_select_bars(pdev, IORESOURCE_IO);
...
- pci_release_selected_regions(pdev, bars);
+ pci_release_io_regions(pdev);


Johannes Thumshirn (5):
  PCI: Add helpers to request/release memory and I/O regions
  NVMe: Use pci_(request|release)_mem_regions
  scsi: Use pci_(request|release)_mem_regions
  GenWQE: Use pci_(request|release)_mem_regions
  ethernet: Use pci_(request|release)_mem_regions

 drivers/misc/genwqe/card_base.c   | 13 +---
 drivers/net/ethernet/atheros/alx/main.c   | 12 +--
 drivers/net/ethernet/intel/e1000e/netdev.c|  6 ++
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 +++---
 drivers/net/ethernet/intel/i40e/i40e_main.c   |  9 +++--
 drivers/net/ethernet/intel/igb/igb_main.c | 10 +++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |  9 +++--
 drivers/nvme/host/pci.c   | 10 +++--
 drivers/scsi/lpfc/lpfc_init.c |  5 +
 include/linux/pci.h   | 29 +++
 10 files changed, 57 insertions(+), 57 deletions(-)

Cc: Christoph Hellwig <h...@infradead.org>
Cc: Keith Busch <keith.bu...@intel.com>
Cc: Jens Axboe <ax...@fb.com>
Cc: linux-n...@lists.infradead.org
Cc: James Smart <james.sm...@avagotech.com>
Cc: Dick Kennedy <dick.kenn...@avagotech.com>
Cc: "James E.J. Bottomley" <j...@linux.vnet.ibm.com>
Cc: "Martin K. Petersen" <martin.peter...@oracle.com>
Cc: linux-s...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: Frank Haverkamp <ha...@linux.vnet.ibm.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Jay Cliburn <jclib...@gmail.com>
Cc: Chris Snook <chris.sn...@gmail.com>
Cc: Jeff Kirsher <jeffrey.t.kirs...@intel.com>
Cc: David S. Miller <da...@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: intel-wired-...@lists.osuosl.org
-- 
1.8.5.6



[PATCH] macvtap: Destroy minor_idr on module_exit

2015-07-08 Thread Johannes Thumshirn
Destroy minor_idr on module_exit, reclaiming the allocated memory.

This was detected by the following semantic patch (written by Luis Rodriguez
mcg...@suse.com)
SmPL
@ defines_module_init @
declarer name module_init, module_exit;
declarer name DEFINE_IDR;
identifier init;
@@

module_init(init);

@ defines_module_exit @
identifier exit;
@@

module_exit(exit);

@ declares_idr depends on defines_module_init  defines_module_exit @
identifier idr;
@@

DEFINE_IDR(idr);

@ on_exit_calls_destroy depends on declares_idr  defines_module_exit @
identifier declares_idr.idr, defines_module_exit.exit;
@@

exit(void)
{
 ...
 idr_destroy(idr);
 ...
}

@ missing_module_idr_destroy depends on declares_idr  defines_module_exit  
!on_exit_calls_destroy @
identifier declares_idr.idr, defines_module_exit.exit;
@@

exit(void)
{
 ...
 +idr_destroy(idr);
}
/SmPL

Signed-off-by: Johannes Thumshirn jthumsh...@suse.de
---
 drivers/net/macvtap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index f837080..3b933bb 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -1355,6 +1355,7 @@ static void macvtap_exit(void)
class_unregister(macvtap_class);
cdev_del(macvtap_cdev);
unregister_chrdev_region(macvtap_major, MACVTAP_NUM_DEVS);
+   idr_destroy(minor_idr);
 }
 module_exit(macvtap_exit);
 
-- 
2.4.3

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html