linux-kernel - Re: [PATCH/RFC 2.6.21 2/5] ehca: ehca

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20070111194000.GE24623@infradead.org>
Date:	Thu, 11 Jan 2007 19:40:00 +0000
From:	Christoph Hellwig <hch@...radead.org>
To:	Hoang-Nam Nguyen <hnguyen@...ux.vnet.ibm.com>
Cc:	Roland Dreier <rdreier@...co.com>, hch@...radead.org,
	linux-kernel@...r.kernel.org, linuxppc-dev@...abs.org,
	openfabrics-ewg@...nib.org, openib-general@...nib.org,
	raisch@...ibm.com
Subject: Re: [PATCH/RFC 2.6.21 2/5] ehca: ehca_uverbs.c: "proper" use of mmap

On Thu, Jan 11, 2007 at 08:08:15PM +0100, Hoang-Nam Nguyen wrote:
> +static void mm_open(struct vm_area_struct *vma)

This should be name ehca_vma_open, dito for mm_close/ehca_vma_close and
vm_ops/ehca_vm_ops.

> +	u32 *count = (u32*)vma->vm_private_data;

No need for the cast here (both in the open and close routine)

> +	for (ofs = 0; ofs < queue->queue_length; ofs += PAGE_SIZE) {
> +		u64 virt_addr = (u64)ipz_qeit_calc(queue, ofs);
> +		page = virt_to_page(virt_addr);
> +		rc = vm_insert_page(vma, start, page);
> +		if (unlikely(rc)) {
> +			ehca_gen_err("vm_insert_page() failed rc=%x", rc);
> +			return rc;
> +		}
> +		start +=  PAGE_SIZE;

Not required for now, but long term you really should rework your
whole queue abstraction to operate on an array of struct pages, that
makes things like this and various other bits in ipz_pt_fn.[ch] a lot
simpler.

>  int ehca_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
>  {

Can you split this monster routine into individual functions for
each type of mmap please?  With two helpers to get and verify the cq/qp
shared by the individual sub-variants, that would also help to get rid
of all those magic offsets.

Actually, this routine directly comes from ib_device.mmap - Roland,
can you shed some light on what's going on here?

Also after applying this patch I have a prototype and various callers
for ehca_mmap_nopage but no actual implementation.  Could it be that
there are some bits missing?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/