lkml.org 
[lkml]   [2006]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 5/6] FDPIC: Add coredump capability for the ELF-FDPIC binfmt [try #3]
    On Thu, 06 Jul 2006 13:47:27 +0100
    David Howells <dhowells@redhat.com> wrote:

    > From: David Howells <dhowells@redhat.com>
    >
    > Add coredump capability for the ELF-FDPIC binfmt.
    >
    > ..
    >
    > +static int dump_seek(struct file *file, off_t off)
    > +{
    > + if (file->f_op->llseek) {
    > + if (file->f_op->llseek(file, off, 0) != off)
    > + return 0;
    > + } else {
    > + file->f_pos = off;
    > + }
    > + return 1;
    > +}

    llseek takes a loff_t and file->f_pos is loff_t. I guess it's a bit moot
    on such a CPU. Was it deliberate?

    (how come the kernel doesn't have a SEEK_SET #define?)

    > +/*
    > + * Decide whether a segment is worth dumping; default is yes to be
    > + * sure (missing info is worse than too much; etc).
    > + * Personally I'd include everything, and use the coredump limit...
    > + *
    > + * I think we should skip something. But I am not sure how. H.J.
    > + */
    > +static inline int maydump(struct vm_area_struct *vma)
    > +{
    > + /* Do not dump I/O mapped devices or special mappings */
    > + if (vma->vm_flags & (VM_IO | VM_RESERVED)) {
    > + kdcore("%08lx: %08lx: no (IO)", vma->vm_start, vma->vm_flags);
    > + return 0;
    > + }
    > +
    > + /* If we may not read the contents, don't allow us to dump
    > + * them either. "dump_write()" can't handle it anyway.
    > + */
    > + if (!(vma->vm_flags & VM_READ)) {
    > + kdcore("%08lx: %08lx: no (!read)", vma->vm_start, vma->vm_flags);
    > + return 0;
    > + }
    > +
    > + /* Dump shared memory only if mapped from an anonymous file. */
    > + if (vma->vm_flags & VM_SHARED) {
    > + if (vma->vm_file->f_dentry->d_inode->i_nlink == 0) {
    > + kdcore("%08lx: %08lx: no (share)", vma->vm_start, vma->vm_flags);
    > + return 1;
    > + }
    > +
    > + kdcore("%08lx: %08lx: no (share)", vma->vm_start, vma->vm_flags);
    > + return 0;
    > + }
    > +
    > +#ifdef CONFIG_MMU
    > + /* If it hasn't been written to, don't write it out */
    > + if (!vma->anon_vma) {
    > + kdcore("%08lx: %08lx: no (!anon)", vma->vm_start, vma->vm_flags);
    > + return 0;
    > + }
    > +#endif
    > +
    > + kdcore("%08lx: %08lx: yes", vma->vm_start, vma->vm_flags);
    > + return 1;
    > +}

    Three callsites - seems too large to inline.

    > +#define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))

    The GFS2 tree has

    --- a/include/linux/kernel.h
    +++ b/include/linux/kernel.h
    @@ -32,6 +32,7 @@ #define STACK_MAGIC 0xdeadbeef

    #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
    #define ALIGN(x,a) (((x)+(a)-1)&~((a)-1))
    +#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
    #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f))

    #define KERN_EMERG "<0>" /* system is unusable */
    Which seems reasonable to me. I'll steal it from them.

    > +/* #define DEBUG */
    > +
    > +#define DUMP_WRITE(addr, nr) \
    > + do { if (!dump_write(file, (addr), (nr))) return 0; } while(0)
    > +#define DUMP_SEEK(off) \
    > + do { if (!dump_seek(file, (off))) return 0; } while(0)
    >
    > ...
    >
    > +#undef DUMP_WRITE
    > +#undef DUMP_SEEK
    > +
    > +#define DUMP_WRITE(addr, nr) \
    > + if ((size += (nr)) > limit || !dump_write(file, (addr), (nr))) \
    > + goto end_coredump;
    > +#define DUMP_SEEK(off) \
    > + if (!dump_seek(file, (off))) \
    > + goto end_coredump;

    Embedding returns and gotos in macros is evil. For new code it's worth
    doing it vaguely tastefully.

    ret = dump_write(...);
    if (ret < 0)
    goto actually_return_an_error_code;
    > + for (vml = current->mm->context.vmlist; vml; vml = vml->next)
    > + segs++;

    Does this need locking?


    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-07-06 19:55    [W:7.471 / U:0.056 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site